Re: [FFmpeg-devel] [PATCH] ffmpeg: fix setting field_order during muxing

2017-08-10 Thread Dave Rice

> On Aug 10, 2017, at 5:18 PM, Jerome Martinez  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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffmpeg: fix setting field_order during muxing

2017-08-10 Thread Jerome Martinez

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.


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffmpeg: fix setting field_order during muxing

2017-08-10 Thread James Almer
On 8/10/2017 5:21 AM, Hendrik Leppkes wrote:
> On Thu, Aug 10, 2017 at 4:43 AM, James Almer  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
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffmpeg: fix setting field_order during muxing

2017-08-10 Thread Hendrik Leppkes
On Thu, Aug 10, 2017 at 4:43 AM, James Almer  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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] ffmpeg: fix setting field_order during muxing

2017-08-09 Thread James Almer
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 
---
 ffmpeg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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;
 
-- 
2.13.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel