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

[David Conrad]

On May 10, 2009, at 2:16 PM, Michael Niedermayer wrote:

> On Sun, May 10, 2009 at 12:10:20AM -0400, David Conrad wrote:
>> On May 9, 2009, at 11:04 PM, Michael Niedermayer wrote:
>>
>>> 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 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
>>
>> Fixed.
>>
>
>> commit a6a181cf9bbc78c6295f1addb5a255ea3f962880
>> 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
>
> ok

Applied.