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

[Philip Langdale]

On Tue, 8 Feb 2011 09:56:30 +0100
Benoit Fouet <benoit.fouet at free.fr> wrote:

> Hi,
> 
> mostly minor remarks. But I think you should now wait for experts to
> comment before bothering sending another patch.

Thanks again. I've made all changes that I don't explicitly reply to.

> 
> OK, this could be done with "OpaqueList **node = &priv->head" to avoid
> the special case; not sure this is really worth it though.

I don't think so; leaving as is.
 
> 
> BC_MEDIA_SUBTYPE? Maybe even use format.mSubtype directly?

Switched type, but still using the local. I've checked in 70012
detection logic and used it to fail out for divx content, so I
have to compare the subtype a few times.
 
> 
> maybe an error label with this uninit+return pattern could be added.
> also, this -1 should be changed to a "real" error code.

Used an error label (wasn't originally sure if that was approved style).
Haven't changed the error code as there's no suitable generic one, and
I don't want to write a mapping function from BC_STS_STATUS and they're
not all mappable anyway.
 
> 
> It looks strange to me to not have the success case as the first one.
> 

I did that in 'chronological' order. You always get an FMT_CHANGE before
the first frame.

--phil