[FFmpeg-devel] [PATCH] Set generic yuv420 sampling in yuv4mpeg by default

[Michael Niedermayer]

On Sat, May 09, 2009 at 08:05:02PM -0400, David Conrad wrote:
> On May 9, 2009, at 6:29 AM, Michael Niedermayer wrote:
>
>> On Sat, May 09, 2009 at 05:48:30AM -0400, David Conrad wrote:
>>> On May 8, 2009, at 9:49 AM, Michael Niedermayer wrote:
>>>
>>>> On Thu, May 07, 2009 at 11:43:25PM -0400, David Conrad wrote:
>>>>> Hi,
>>>>>
>>>>> As discovered in #x264, the dump_psnr tool in Thusnelda actually cares
>>>>> about chroma sample positions. This should probably be more correct; I
>>>>> don't know for sure which codecs define which positioning.
>>>>
>>>> many codecs leave it unspecified
>>>> h264 & mpeg4 match mpeg2 by default in theory, in how far the actual
>>>> data matches this of course is another question
>>>
>>> I didn't know that; but probably the correlation with actual data isn't
>>> much worse than mpeg2.
>>
>> maybe
>>
>>
>>>
>>>> h264 allows the exact chroma positions to be stored for 420
>>>> we could add a chroma_sample_location field to AVCodecContext maybe
>>>
>>> I wound up doing this and adding values for a couple codecs I looked up.
>>>
>>
>>> commit 1c69912aac082eb182e619fa75c127823167d8ed
>>> Author: David Conrad <lessen42 at gmail.com>
>>> Date:   Sat May 9 05:41:16 2009 -0400
>>>
>>>    Add a chroma_sample_location field to define positioning of chroma 
>>> samples
>>>
>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>> index 4226673..b6c6cc4 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -2481,6 +2481,20 @@ typedef struct AVCodecContext {
>>>      * - decoding: Set by libavcodec
>>>      */
>>>     enum AVColorRange color_range;
>>> +
>>> +    /**
>>> +     * This defines the location of chroma samples.
>>> +     * If the 2nd element is nonzero, it defines the location of
>>> +     * bottom field chroma while the 1st element defines top field 
>>> chroma.
>>> +     *
>>> +     *  X   X        3 4 X      X are luma samples,
>>> +     *               1 2        1-6 are possible chroma positions
>>> +     *  X   X        5 6 X      0 is undefined/unknown position
>>> +     *
>>> +     * - encoding: Set by user
>>> +     * - decoding: Set by libavcodec
>>> +     */
>>> +     int chroma_sample_location[2];
>>
>> Isnt array[2] overkill?
>> i know h264 can store these seperately for both fields but it seems really
>> silly, i mean what could one do with the data if the chroma sample 
>> positions
>> of 2 fields missmatched ...
>> besides what insane hardware would generate such stuff ...
>
> No clue.. I just wasn't sure whether to ignore the top or bottom for h264.
>

> commit f08dcb90cda6fa44c91cdefa917d2dd0d57cd497
> Author: David Conrad <lessen42 at gmail.com>
> Date:   Sat May 9 19:57:11 2009 -0400
> 
>     Add a chroma_sample_location field to define positioning of chroma samples
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 4226673..a733a33 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -479,6 +479,21 @@ enum AVColorRange{
>      AVCOL_RANGE_NB           , ///< Not part of ABI
>  };
>  
> +/**
> + *  X   X      3 4 X      X are luma samples,
> + *             1 2        1-6 are possible chroma positions
> + *  X   X      5 6 X      0 is undefined/unknown position
> + */
> +enum AVChromaLocation{
> +    AVCHROMA_LOC_UNSPECIFIED=0,
> +    AVCHROMA_LOC_LEFT       =1, ///< mpeg2/4, h264 default
> +    AVCHROMA_LOC_CENTER     =2, ///< mpeg1, jpeg, h263
> +    AVCHROMA_LOC_TOPLEFT    =3, ///< DV
> +    AVCHROMA_LOC_TOP        =4,
> +    AVCHROMA_LOC_BOTTOMLEFT =5,
> +    AVCHROMA_LOC_BOTTOM     =6,
> +};
> +
>  typedef struct RcOverride{
>      int start_frame;
>      int end_frame;
> @@ -2481,6 +2496,13 @@ typedef struct AVCodecContext {
>       * - decoding: Set by libavcodec
>       */
>      enum AVColorRange color_range;
> +
> +    /**
> +     * This defines the location of chroma samples.
> +     * - encoding: Set by user
> +     * - decoding: Set by libavcodec
> +     */
> +     enum AVChromaLocation chroma_sample_location;
>  } AVCodecContext;

update to the AVOptions array is missing

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is not what we do, but why we do it that matters.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090510/c08f9690/attachment.pgp>