| Message ID | 20210410210023.27057-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avcodec/h264dec: add missing flags to is_avc and nal_length_size AVOptions | expand |
| 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 |
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?
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.
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 --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
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(-)