bcm2835-codec tweaks

[6by9]

No description provided.

[6by9]

@jc-kynesim I did go for the framework tweak instead of totally reworking the buffer feed code.

@popcornmix Hopefully pixel aspect ratio reporting.

[pelwell]

You lost me in the second commit, but here's some feedback.

[pelwell]

I may have misunderstood the intention of this commit, but it appears to bypass the streaming check if running in buffered mode.

[6by9]

You're right, but it wasn't quite my intent. Too much inverted logic.

I wanted the existing logic when not buffering, but to not return should either queue be streaming when buffered. One to rework.

[6by9]

Updated to hopefully address your comments.

[popcornmix]

Kodi doesn't see the aspect ratio, but it might be getting lost in ffmpeg or kodi itself. I'll see if I can find it...

[6by9]

Does Kodi / FFmpeg actually make use of the resolution changed event? If it has configured the output correctly then there is no event and we have no chance to report the PAR.

[pelwell]

I just spent a minute or two writing out your condition as you described it:

	if (!((m2m_ctx->out_q_ctx.q.streaming &&
	       m2m_ctx->cap_q_ctx.q.streaming) ||
	      (m2m_ctx->out_q_ctx.buffered && m2m_ctx->out_q_ctx.q.streaming))) {

and then using De Morgan's laws to transform it into what you have written. But I shouldn't have to - what's wrong with the literal version above?

[blazczak]

The OP's version is just an evolution of pre-existing (and thus assumed stable to some degree through testing/deployment) code using a minimal set of changes.

The check could also be written as

	if (!(m2m_ctx->out_q_ctx.q.streaming && m2m_ctx-cap_q_ctx.q.streaming) && 
	       !(m2m_ctx->out_q_ctx.buffered && m2m_ctx->out_q_ctx.q.streaming)) {

They are all logically equivalent but having OR inside the clause and top-level NOT in the proposed transformed version depending on the conditions could make it a slower performing check if no short-circuiting happens inside and the result is not final until after the negation is performed at the end.

The changes suggested in c54a951 still employ a short-circuit, so they look ok to me.

[pelwell]

They aren't wrong, but in (especially) kernel work clarity trumps pretty much everything, especially when the compiler will likely generate the same code in both cases.

[blazczak]

I agree on clarity, thus my suggestion above. Of course, as more conditions get added, the clause may have to be rewritten again.

[6by9]

I was just retaining the format of the original term. I'll rework as requested.

[6by9]

I've dropped the patch for enabling the decoder on STREAMON(OUTPUT) for now, but done the v4l2_m2m rework that was requested.
This has also gained a patch to enable VC1. It needs a firmware change to actually work reliably, but is harmless to merge this side first.

[popcornmix]

I'm happy with this. Aspect ratio does work (needs an ffmpeg patch).
And VC1 is working.

[popcornmix]

As mentioned on slack the VC1 patch doesn't support WMV9 (and the hardware/omplayer do).
So might be worth checking that before merging.

[6by9]

As mentioned on slack the VC1 patch doesn't support WMV9 (and the hardware/omplayer do).
So might be worth checking that before merging.

Not going to happen in the short term. The codec is getting upset at quite a low level with that.
It needs a fair amount of analysis and bitstream parsing so that RIL can strip off the headers and then still pass the start of the stream into the codec for decoding. WMV is all special cases in RIL for that :-(

[popcornmix]

Okay, I'm happy with PR then.