| Message ID | 20170810024336.7540-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Aug 10, 2017 at 4:43 AM, James Almer <jamrial@gmail.com> wrote: > AVFrame.top_field_first doxy states > > "If the content is interlaced, is top field displayed first." > > And AVFieldOrder doxy defines: > AV_FIELD_TB, //< Top coded first, bottom displayed first > AV_FIELD_BT, //< Bottom coded first, top displayed first > > Fixes ticket #6577 > Isn't top coded first in most codecs? So maybe it should use TT (top coded and displayed first), and TB (top coded first, bottom displayed first)? I suppose that difference between coding order might be highly codec dependent, though. - Hendrik
On 8/10/2017 5:21 AM, Hendrik Leppkes wrote: > On Thu, Aug 10, 2017 at 4:43 AM, James Almer <jamrial@gmail.com> wrote: >> AVFrame.top_field_first doxy states >> >> "If the content is interlaced, is top field displayed first." >> >> And AVFieldOrder doxy defines: >> AV_FIELD_TB, //< Top coded first, bottom displayed first >> AV_FIELD_BT, //< Bottom coded first, top displayed first >> >> Fixes ticket #6577 >> > > Isn't top coded first in most codecs? So maybe it should use TT (top > coded and displayed first), and TB (top coded first, bottom displayed > first)? I can make it TT/TB if that's preferred, but it would be as much of a guess as it currently is. > I suppose that difference between coding order might be highly codec > dependent, though. I don't know. ffmpeg.c right now is using TT/BB for mjpeg only, so it has at least one codec specific case. A quick grep shows codecs like h264, vc1 and canopus using TT/BB, and ffv1 using TT/TB, so someone more familiar with this might want to take a look at it and add some other cases as required, at least for codecs supported in containers that care about the contents of AVFormatContext.field_order (matroska, mov, etc). > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Le 10/08/2017 à 04:43, James Almer a écrit : > AVFrame.top_field_first doxy states > > "If the content is interlaced, is top field displayed first." > > And AVFieldOrder doxy defines: > AV_FIELD_TB, //< Top coded first, bottom displayed first > AV_FIELD_BT, //< Bottom coded first, top displayed first > > Fixes ticket #6577 IMHO the subject is complex, and everyone should be in sync with the purpose of each mux_par->field_order value and understand impact on other players (sometimes the MOV metadata is the only one available as the codec has no info e.g. uncompressed or jp2k) before changing current behavior in a so general manner. What I understood is that it is based on QuickTime specs (the mapping is 1 to 1 from fiel atom) and TN2162 and that they are not easy to understand and maybe misleading : the feedback I got in the last years about the meaning of this atom are not really in sync with specs having stored vs displayed, and such change may have an impact on how other players handle fiel atom or other metadata in the way it is currently implemented by FFmpeg.
> On Aug 10, 2017, at 5:18 PM, Jerome Martinez <Jerome@MediaArea.net> wrote: > > Le 10/08/2017 à 04:43, James Almer a écrit : >> AVFrame.top_field_first doxy states >> >> "If the content is interlaced, is top field displayed first." >> >> And AVFieldOrder doxy defines: >> AV_FIELD_TB, //< Top coded first, bottom displayed first >> AV_FIELD_BT, //< Bottom coded first, top displayed first >> >> Fixes ticket #6577 > > IMHO the subject is complex, and everyone should be in sync with the purpose of each mux_par->field_order value and understand impact on other players (sometimes the MOV metadata is the only one available as the codec has no info e.g. uncompressed or jp2k) before changing current behavior in a so general manner. > What I understood is that it is based on QuickTime specs (the mapping is 1 to 1 from fiel atom) and TN2162 and that they are not easy to understand and maybe misleading : the feedback I got in the last years about the meaning of this atom are not really in sync with specs having stored vs displayed, and such change may have an impact on how other players handle fiel atom or other metadata in the way it is currently implemented by FFmpeg. I filed ticket #6577 but not feel cautious about this patch and consider that my initial report may have been based on some misunderstanding. As Jerome notes the MKV field order flag is derived from mov's fiel atom and there are two definitions for that: https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html (Table 4-2) https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG10-THE__FIEL__IMAGEDESCRIPTION_EXTENSION__FIELD_FRAME_INFORMATION (Figure 4) From testing we have found that ffmpeg's behavior here is matching some Apple mov muxers in the assignment of a field order value, so the patch may be unneeded or the reason may need more testing. Dave Rice
diff --git a/ffmpeg.c b/ffmpeg.c index 888d19a647..a08db61ea3 100644 --- a/ffmpeg.c +++ b/ffmpeg.c @@ -1206,7 +1206,7 @@ static void do_video_out(OutputFile *of, avoid any copies. We support temporarily the older method. */ if (in_picture->interlaced_frame) - mux_par->field_order = in_picture->top_field_first ? AV_FIELD_TB:AV_FIELD_BT; + mux_par->field_order = in_picture->top_field_first ? AV_FIELD_BT:AV_FIELD_TB; else mux_par->field_order = AV_FIELD_PROGRESSIVE; pkt.data = (uint8_t *)in_picture; @@ -1229,7 +1229,7 @@ static void do_video_out(OutputFile *of, if (enc->codec->id == AV_CODEC_ID_MJPEG) mux_par->field_order = in_picture->top_field_first ? AV_FIELD_TT:AV_FIELD_BB; else - mux_par->field_order = in_picture->top_field_first ? AV_FIELD_TB:AV_FIELD_BT; + mux_par->field_order = in_picture->top_field_first ? AV_FIELD_BT:AV_FIELD_TB; } else mux_par->field_order = AV_FIELD_PROGRESSIVE;
AVFrame.top_field_first doxy states "If the content is interlaced, is top field displayed first." And AVFieldOrder doxy defines: AV_FIELD_TB, //< Top coded first, bottom displayed first AV_FIELD_BT, //< Bottom coded first, top displayed first Fixes ticket #6577 Signed-off-by: James Almer <jamrial@gmail.com> --- ffmpeg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)