diff mbox series

[FFmpeg-devel,1/4] avcodec/h264dec: add missing flags to is_avc and nal_length_size AVOptions

Message ID 20210410210023.27057-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/4] avcodec/h264dec: add missing flags to is_avc and nal_length_size AVOptions
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

James Almer April 10, 2021, 9 p.m. UTC
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/h264dec.c      | 5 +++--
 tests/ref/fate/mov-zombie | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

Anton Khirnov April 11, 2021, 9:56 a.m. UTC | #1
Quoting James Almer (2021-04-10 23:00:20)
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/h264dec.c      | 5 +++--
>  tests/ref/fate/mov-zombie | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 47b9abbc5c..f44c8c8175 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -1027,9 +1027,10 @@ static int h264_decode_frame(AVCodecContext *avctx, void *data,
>  
>  #define OFFSET(x) offsetof(H264Context, x)
>  #define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
> +#define VDE VD | AV_OPT_FLAG_EXPORT
>  static const AVOption h264_options[] = {
> -    { "is_avc", "is avc", OFFSET(is_avc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, 0 },
> -    { "nal_length_size", "nal_length_size", OFFSET(nal_length_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, 0 },
> +    { "is_avc", "is avc", OFFSET(is_avc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VDE },
> +    { "nal_length_size", "nal_length_size", OFFSET(nal_length_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, VDE },

Not objecting to the patch, but does anyone know how these fields are
useful to callers?
James Almer April 11, 2021, 1:25 p.m. UTC | #2
On 4/11/2021 6:56 AM, Anton Khirnov wrote:
> Quoting James Almer (2021-04-10 23:00:20)
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/h264dec.c      | 5 +++--
>>   tests/ref/fate/mov-zombie | 2 +-
>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
>> index 47b9abbc5c..f44c8c8175 100644
>> --- a/libavcodec/h264dec.c
>> +++ b/libavcodec/h264dec.c
>> @@ -1027,9 +1027,10 @@ static int h264_decode_frame(AVCodecContext *avctx, void *data,
>>   
>>   #define OFFSET(x) offsetof(H264Context, x)
>>   #define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
>> +#define VDE VD | AV_OPT_FLAG_EXPORT
>>   static const AVOption h264_options[] = {
>> -    { "is_avc", "is avc", OFFSET(is_avc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, 0 },
>> -    { "nal_length_size", "nal_length_size", OFFSET(nal_length_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, 0 },
>> +    { "is_avc", "is avc", OFFSET(is_avc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VDE },
>> +    { "nal_length_size", "nal_length_size", OFFSET(nal_length_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, VDE },
> 
> Not objecting to the patch, but does anyone know how these fields are
> useful to callers?

When i asked if these two were meant to be exported at all on IRC, Jan 
Ekström argued that as exported avoptions they could be useful for 
callers to know if the stream being decoded is annexb or size delimited, 
so i figured i might as well keep them as is.
As for being input options, if there's no extradata during init or 
propagated as packet side data, situation where decoder will not 
overwrite them but still use them when splitting NALUs, the caller can 
set them manually and decoding could in theory succeed.

I don't know if the user that opened ticket #9176 cared about and used 
them, or just noticed ffprobe printed different values in his own tests, 
and reported it.
James Almer April 13, 2021, 11:24 p.m. UTC | #3
On 4/11/2021 10:25 AM, James Almer wrote:
> On 4/11/2021 6:56 AM, Anton Khirnov wrote:
>> Quoting James Almer (2021-04-10 23:00:20)
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>>   libavcodec/h264dec.c      | 5 +++--
>>>   tests/ref/fate/mov-zombie | 2 +-
>>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
>>> index 47b9abbc5c..f44c8c8175 100644
>>> --- a/libavcodec/h264dec.c
>>> +++ b/libavcodec/h264dec.c
>>> @@ -1027,9 +1027,10 @@ static int h264_decode_frame(AVCodecContext 
>>> *avctx, void *data,
>>>   #define OFFSET(x) offsetof(H264Context, x)
>>>   #define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
>>> +#define VDE VD | AV_OPT_FLAG_EXPORT
>>>   static const AVOption h264_options[] = {
>>> -    { "is_avc", "is avc", OFFSET(is_avc), AV_OPT_TYPE_BOOL, {.i64 = 
>>> 0}, 0, 1, 0 },
>>> -    { "nal_length_size", "nal_length_size", OFFSET(nal_length_size), 
>>> AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, 0 },
>>> +    { "is_avc", "is avc", OFFSET(is_avc), AV_OPT_TYPE_BOOL, {.i64 = 
>>> 0}, 0, 1, VDE },
>>> +    { "nal_length_size", "nal_length_size", OFFSET(nal_length_size), 
>>> AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, VDE },
>>
>> Not objecting to the patch, but does anyone know how these fields are
>> useful to callers?
> 
> When i asked if these two were meant to be exported at all on IRC, Jan 
> Ekström argued that as exported avoptions they could be useful for 
> callers to know if the stream being decoded is annexb or size delimited, 
> so i figured i might as well keep them as is.
> As for being input options, if there's no extradata during init or 
> propagated as packet side data, situation where decoder will not 
> overwrite them but still use them when splitting NALUs, the caller can 
> set them manually and decoding could in theory succeed.
> 
> I don't know if the user that opened ticket #9176 cared about and used 
> them, or just noticed ffprobe printed different values in his own tests, 
> and reported it.

Will apply.
diff mbox series

Patch

diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 47b9abbc5c..f44c8c8175 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -1027,9 +1027,10 @@  static int h264_decode_frame(AVCodecContext *avctx, void *data,
 
 #define OFFSET(x) offsetof(H264Context, x)
 #define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
+#define VDE VD | AV_OPT_FLAG_EXPORT
 static const AVOption h264_options[] = {
-    { "is_avc", "is avc", OFFSET(is_avc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, 0 },
-    { "nal_length_size", "nal_length_size", OFFSET(nal_length_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, 0 },
+    { "is_avc", "is avc", OFFSET(is_avc), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, VDE },
+    { "nal_length_size", "nal_length_size", OFFSET(nal_length_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, VDE },
     { "enable_er", "Enable error resilience on damaged frames (unsafe)", OFFSET(enable_er), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD },
     { "x264_build", "Assume this x264 version if no x264 version found in any SEI", OFFSET(x264_build), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, VD },
     { NULL },
diff --git a/tests/ref/fate/mov-zombie b/tests/ref/fate/mov-zombie
index 7b417e59dd..1b54c58d04 100644
--- a/tests/ref/fate/mov-zombie
+++ b/tests/ref/fate/mov-zombie
@@ -194,5 +194,5 @@  frame|media_type=video|stream_index=0|key_frame=0|pkt_pts=188623|pkt_pts_time=2.
 packet|codec_type=video|stream_index=0|pts=197632|pts_time=2.195911|dts=191625|dts_time=2.129167|duration=3003|duration_time=0.033367|size=580|pos=101820|flags=__
 frame|media_type=video|stream_index=0|key_frame=0|pkt_pts=191626|pkt_pts_time=2.129178|pkt_dts=N/A|pkt_dts_time=N/A|best_effort_timestamp=191626|best_effort_timestamp_time=2.129178|pkt_duration=3003|pkt_duration_time=0.033367|pkt_pos=99180|pkt_size=1666|width=160|height=240|pix_fmt=yuv420p|sample_aspect_ratio=2:1|pict_type=P|coded_picture_number=63|display_picture_number=0|interlaced_frame=0|top_field_first=0|repeat_pict=0|color_range=tv|color_space=smpte170m|color_primaries=smpte170m|color_transfer=bt709|chroma_location=topleftside_data|side_data_type=H.26[45] User Data Unregistered SEI message
 
-stream|index=0|codec_name=h264|profile=77|codec_type=video|codec_tag_string=avc1|codec_tag=0x31637661|width=160|height=240|coded_width=160|coded_height=240|closed_captions=0|has_b_frames=1|sample_aspect_ratio=2:1|display_aspect_ratio=4:3|pix_fmt=yuv420p|level=12|color_range=tv|color_space=smpte170m|color_transfer=bt709|color_primaries=smpte170m|chroma_location=topleft|field_order=unknown|refs=2|is_avc=true|nal_length_size=4|id=N/A|r_frame_rate=30000/1001|avg_frame_rate=6372000/212521|time_base=1/90000|start_pts=0|start_time=0.000000|duration_ts=2125200|duration=23.613333|bit_rate=333874|max_bit_rate=N/A|bits_per_raw_sample=8|nb_frames=708|nb_read_frames=65|nb_read_packets=66|disposition:default=1|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0|tag:rotate=0|tag:creation_time=2008-05-12T20:59:27.000000Z|tag:language=eng|tag:handler_name=Apple Video Media Handler|tag:vendor_id=appl|tag:encoder=H.264
+stream|index=0|codec_name=h264|profile=77|codec_type=video|codec_tag_string=avc1|codec_tag=0x31637661|width=160|height=240|coded_width=160|coded_height=240|closed_captions=0|has_b_frames=1|sample_aspect_ratio=2:1|display_aspect_ratio=4:3|pix_fmt=yuv420p|level=12|color_range=tv|color_space=smpte170m|color_transfer=bt709|color_primaries=smpte170m|chroma_location=topleft|field_order=unknown|refs=2|id=N/A|r_frame_rate=30000/1001|avg_frame_rate=6372000/212521|time_base=1/90000|start_pts=0|start_time=0.000000|duration_ts=2125200|duration=23.613333|bit_rate=333874|max_bit_rate=N/A|bits_per_raw_sample=8|nb_frames=708|nb_read_frames=65|nb_read_packets=66|disposition:default=1|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0|tag:rotate=0|tag:creation_time=2008-05-12T20:59:27.000000Z|tag:language=eng|tag:handler_name=Apple Video Media Handler|tag:vendor_id=appl|tag:encoder=H.264
 side_data|side_data_type=Display Matrix|displaymatrix=\n00000000:       131072           0           0\n00000001:            0       65536           0\n00000002:            0           0  1073741824\n|rotation=0