[FFmpeg-devel] libavcodec: fix field_order labelling

Submitted by Dave Rice on Aug. 12, 2017, 4:47 p.m.

Details

Message ID F4FE4E00-8A8E-40A0-9421-97020A7D6E54@dericed.com
State New
Headers show

Commit Message

Dave Rice Aug. 12, 2017, 4:47 p.m.
Hello all,
This issue originated in this thread https://github.com/amiaopensource/vrecord/issues/170. On Field Order, in the QuickTime specification at https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html (and similarly in the Matroska specification which adopted similar language) it states the following meanings for field order values:

> 9 – B is displayed earliest, T is stored first in the file. 14 – T is displayed earliest, B is stored first in the file.

 This definition is contradicted by other Apple documentation such as https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG10-THE__FIEL__IMAGEDESCRIPTION_EXTENSION__FIELD_FRAME_INFORMATION. 

An Apple engineer confirmed that the QuickTime specification’s definitions for those Field Order values is wrong and does not match Apple’s (of FFmpeg’s) practice, see https://github.com/amiaopensource/vrecord/issues/170#issuecomment-321937668.

However I think that some of the commenting in ffmpeg is based upon the inaccurate definitions from Apple. For instance, in that thread David Singer confirms:

> Ah, not quite. 1 and 6 are indeed 'planar' (all of one field before all of the other). They don't concern us. Both 9 and 14 are stored in spatial order (i.e. you could do terrible de-interlacing by simply displaying the buffer as a frame), and the 9 or 14 value tells you which field is to be displayed first.
> 
> 9 – T is earlier than B. 14 – B is earlier than T

mov.c associates AV_FIELD_TB with 9 and AV_FIELD_BT with 14 (similar associations in matroska.h), but avcodec.h states:

> AV_FIELD_TB,          //< Top coded first, bottom displayed first
> AV_FIELD_BT,          //< Bottom coded first, bottom displayed first

IMHO in both cases of AV_FIELD_TB and AV_FIELD_BT the coding should be referred as interleaved rather than ‘bottom coded first’ or ‘top coded first’. In the case of AV_FIELD_TT and AV_FIELD_BB the fields are stored as planar images where storage order is notable, but with TB and BT the fields are interleaved.

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@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(-)

Comments

Alex Converse Aug. 16, 2017, 3:54 a.m.
On Sat, Aug 12, 2017 at 9:47 AM, Dave Rice <dave@dericed.com> wrote:
>
> Hello all,
> This issue originated in this thread https://github.com/amiaopensource/vrecord/issues/170. On Field Order, in the QuickTime specification at https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html (and similarly in the Matroska specification which adopted similar language) it states the following meanings for field order values:
>
> > 9 – B is displayed earliest, T is stored first in the file. 14 – T is displayed earliest, B is stored first in the file.
>
>  This definition is contradicted by other Apple documentation such as https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG10-THE__FIEL__IMAGEDESCRIPTION_EXTENSION__FIELD_FRAME_INFORMATION.
>
> An Apple engineer confirmed that the QuickTime specification’s definitions for those Field Order values is wrong and does not match Apple’s (of FFmpeg’s) practice, see https://github.com/amiaopensource/vrecord/issues/170#issuecomment-321937668.
>
> However I think that some of the commenting in ffmpeg is based upon the inaccurate definitions from Apple. For instance, in that thread David Singer confirms:
>
> > Ah, not quite. 1 and 6 are indeed 'planar' (all of one field before all of the other). They don't concern us. Both 9 and 14 are stored in spatial order (i.e. you could do terrible de-interlacing by simply displaying the buffer as a frame), and the 9 or 14 value tells you which field is to be displayed first.
> >
> > 9 – T is earlier than B. 14 – B is earlier than T
>
> mov.c associates AV_FIELD_TB with 9 and AV_FIELD_BT with 14 (similar associations in matroska.h), but avcodec.h states:
>
> > AV_FIELD_TB,          //< Top coded first, bottom displayed first
> > AV_FIELD_BT,          //< Bottom coded first, bottom displayed first
>
> IMHO in both cases of AV_FIELD_TB and AV_FIELD_BT the coding should be referred as interleaved rather than ‘bottom coded first’ or ‘top coded first’. In the case of AV_FIELD_TT and AV_FIELD_BB the fields are stored as planar images where storage order is notable, but with TB and BT the fields are interleaved.
>
> 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?
>

Icefloe019 agrees with your changes:
http://mirror.informatimago.com/next/developer.apple.com/quicktime/icefloe/dispatch019.html#fiel

They seems reasonable to me. when I made this change originally my
primary concern was getting the field info out of extradata.

Patch hide | download patch | download mbox

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
 };
 
 /**
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 1336e921c9..926c964846 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1406,9 +1406,9 @@  void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
                 else if (enc->field_order == AV_FIELD_BB)
                     field_order = "bottom first";
                 else if (enc->field_order == AV_FIELD_TB)
-                    field_order = "top coded first (swapped)";
+                    field_order = "top first (interleaved)";
                 else if (enc->field_order == AV_FIELD_BT)
-                    field_order = "bottom coded first (swapped)";
+                    field_order = "bottom first (interleaved)";
 
                 av_strlcatf(detail, sizeof(detail), "%s, ", field_order);
             }