[FFmpeg-devel] [PATCH] libavcodec: fix field_order labelling

[Tobias Rapp]

On 27.08.2017 19:13, Marton Balint wrote:
> 
> 
> On Sat, 12 Aug 2017, Dave Rice wrote:
> 
> [..]
> 
>> Also utils.c associates these field order values with the following 
>> labels:
>>
>>> AV_FIELD_TB  -> "top coded first (swapped)";
>>> AV_FIELD_BT -> "bottom coded first (swapped)";
>>
>> From my reading, I infer that "top coded first (swapped)” means "top 
>> coded first, bottom displayed first”; however in practice from files 
>> generated by QuickTime and FFmpeg files with a value of TB have the 
>> top field displayed first, so I think the labels are swapped. In the 
>> patch below I suggest using “top first (interleaved)” for TB and 
>> “bottom first (interleaved)” for BT.
>>
>> Comments?
>>
>>
>> From de871b3fa891fa0ae6856461c1f8305cc889cde7 Mon Sep 17 00:00:00 2001
>> From: Dave Rice <dave at dericed.com>
>> Date: Sat, 12 Aug 2017 12:30:43 -0400
>> Subject: [PATCH] libavcodec: fix field_order labelling
>>
>> ---
>> libavcodec/avcodec.h | 4 ++--
>> libavcodec/utils.c   | 4 ++--
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index c594993766..37c39072b3 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -1726,8 +1726,8 @@ enum AVFieldOrder {
>>     AV_FIELD_PROGRESSIVE,
>>     AV_FIELD_TT,          //< Top coded_first, top displayed first
>>     AV_FIELD_BB,          //< Bottom coded first, bottom displayed first
>> -    AV_FIELD_TB,          //< Top coded first, bottom displayed first
>> -    AV_FIELD_BT,          //< Bottom coded first, top displayed first
>> +    AV_FIELD_TB,          //< Interleaved coding, top displayed first
>> +    AV_FIELD_BT,          //< Interleaved coding, bottom displayed first
>> };
> 
> I agree that a lot of stuff in the codebase is consistent with the 
> updated descriptions. However, as far as I see libavformat/mxfdec.c 
> seems to follow the existing docs, so I think that needs changing as well.

In mxfdec.c the value of field_order is constructed from VideoLineMap 
and FieldDominance. The VideoLineMap property indicates coded field 
order and the FieldDominance property indicated whether display field 
order is flipped compared to the coded order or not.

So yes, mxfdec.c is following the current documentation of AVFieldOrder 
and applying the patch would conflict with the MXF specs, AFAICS.

CC-ing Jérôme Martinez as he has much more experience with MXF 
real-world file variations.

Regards,
Tobias