[FFmpeg-devel] [PATCH] CrystalHD decoder support v3

[Philip Langdale]

On Mon, 7 Feb 2011 09:01:23 +0100
Benoit Fouet <benoit.fouet at free.fr> wrote:

> Hi,
> 
> > 
> > I've changed the code to make it explicit but these are boolean
> > results, not raw flags.
> >  
> 
> The first one is a boolean, the second one is not, and if
> VDEC_FLAG_BOTTOM_FIRST could not be expressed with a 8-bits variable,
> the bottom_first could be wrong.

Agreed. What I did in v4 is change the code to explicitly make the
result a boolean and no demotion to uint8_t occurs for the actual
calculation.

> > > > +    int width = output->PicInfo.width * 2; // 16bits per pixel
> > > 
> > > Even though it's hardcoded for now, this could maybe depend on the
> > > pixel format?
> > 
> > It does, but the pixel format is hardcoded too, so it's all the
> > same. I've added a comment.
> >  
> 
> Maybe I was unclear, but that was exactly my point: hardcoding the
> pixel format should be enough. Making the width depend on the pixel
> format would allow it to change "automagically" if/when the pixel
> format changes.

I didn't find any function that returns the pixel size for a pix_fmt.
If there is one, I'll certainly use it.
 
> 
> I'd like to hear from other people about those waits.

As would I. One thing worth mentioning is that the main challenge here
is keeping the pipeline depth constant. That means it's really important
to be able to return a field/frame for every decode() call that consumes
a packet, once the pipeline has been established. If we don't use the
waits as they currently are, we'd probably need to wait in a loop for a
frame to become ready, so it ultimately adds up to the same thing. The
one advantage of the current approach is that it doesn't require an
explicit decision to be made as to when the pipeline is established, it
naturally stabilises.

--phil