| Message ID | 20200508020929.68603-1-ollywoodman@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/h264: support sps/pps AV_PKT_DATA_NEW_EXTRADATA | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Do I need to do anything else for someone to take a look? It would be great
to get it merged, if possible, and I think the change itself is pretty
trivial. Apologies if this is being pinged too soon; I'm trying to follow
the guidance on the FFmpeg contributing page ("if some time passes without
reaction, send a reminder by email.").
Thanks!
On Fri, 8 May 2020 at 03:10, Oliver Woodman <ollywoodman@gmail.com> wrote:
> https://github.com/FFmpeg/FFmpeg/commit/601c238 added support
> for AV_PKT_DATA_NEW_EXTRADATA, but only for avcC extradata.
> This commit adds support for sps/pps extradata as well. This
> makes support consistent for passing new extradata consistent
> with the types of extradata that can be passed when initializing
> the decoder.
>
> Signed-off-by: Oliver Woodman <ollywoodman@gmail.com>
> ---
> libavcodec/h264dec.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 4c355feb18..c1d103a474 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -829,7 +829,7 @@ static int output_frame(H264Context *h, AVFrame *dst,
> H264Picture *srcp)
> return 0;
> }
>
> -static int is_extra(const uint8_t *buf, int buf_size)
> +static int is_avcc_extradata(const uint8_t *buf, int buf_size)
> {
> int cnt= buf[5]&0x1f;
> const uint8_t *p= buf+6;
> @@ -956,16 +956,15 @@ static int h264_decode_frame(AVCodecContext *avctx,
> void *data,
> if (buf_size == 0)
> return send_next_delayed_frame(h, pict, got_frame, 0);
>
> - if (h->is_avc && av_packet_get_side_data(avpkt,
> AV_PKT_DATA_NEW_EXTRADATA, NULL)) {
> + if (av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA, NULL)) {
> int side_size;
> uint8_t *side = av_packet_get_side_data(avpkt,
> AV_PKT_DATA_NEW_EXTRADATA, &side_size);
> - if (is_extra(side, side_size))
> - ff_h264_decode_extradata(side, side_size,
> - &h->ps, &h->is_avc,
> &h->nal_length_size,
> - avctx->err_recognition, avctx);
> + ff_h264_decode_extradata(side, side_size,
> + &h->ps, &h->is_avc, &h->nal_length_size,
> + avctx->err_recognition, avctx);
> }
> if (h->is_avc && buf_size >= 9 && buf[0]==1 && buf[2]==0 &&
> (buf[4]&0xFC)==0xFC) {
> - if (is_extra(buf, buf_size))
> + if (is_avcc_extradata(buf, buf_size))
> return ff_h264_decode_extradata(buf, buf_size,
> &h->ps, &h->is_avc,
> &h->nal_length_size,
> avctx->err_recognition,
> avctx);
> --
> 2.26.0.110.g2183baf09c-goog
>
>
On Fri, 8 May 2020 at 03:10, Oliver Woodman <ollywoodman@gmail.com> wrote: > https://github.com/FFmpeg/FFmpeg/commit/601c238 added support > for AV_PKT_DATA_NEW_EXTRADATA, but only for avcC extradata. > This commit adds support for sps/pps extradata as well. This > makes support consistent for passing new extradata consistent > with the types of extradata that can be passed when initializing > the decoder. > > Signed-off-by: Oliver Woodman <ollywoodman@gmail.com> > --- > libavcodec/h264dec.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c > index 4c355feb18..c1d103a474 100644 > --- a/libavcodec/h264dec.c > +++ b/libavcodec/h264dec.c > @@ -829,7 +829,7 @@ static int output_frame(H264Context *h, AVFrame *dst, > H264Picture *srcp) > return 0; > } > > -static int is_extra(const uint8_t *buf, int buf_size) > +static int is_avcc_extradata(const uint8_t *buf, int buf_size) > { > int cnt= buf[5]&0x1f; > const uint8_t *p= buf+6; > @@ -956,16 +956,15 @@ static int h264_decode_frame(AVCodecContext *avctx, > void *data, > if (buf_size == 0) > return send_next_delayed_frame(h, pict, got_frame, 0); > > - if (h->is_avc && av_packet_get_side_data(avpkt, > AV_PKT_DATA_NEW_EXTRADATA, NULL)) { > + if (av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA, NULL)) { > int side_size; > uint8_t *side = av_packet_get_side_data(avpkt, > AV_PKT_DATA_NEW_EXTRADATA, &side_size); > - if (is_extra(side, side_size)) > - ff_h264_decode_extradata(side, side_size, > - &h->ps, &h->is_avc, > &h->nal_length_size, > - avctx->err_recognition, avctx); > + ff_h264_decode_extradata(side, side_size, > + &h->ps, &h->is_avc, &h->nal_length_size, > + avctx->err_recognition, avctx); > } > if (h->is_avc && buf_size >= 9 && buf[0]==1 && buf[2]==0 && > (buf[4]&0xFC)==0xFC) { > - if (is_extra(buf, buf_size)) > + if (is_avcc_extradata(buf, buf_size)) > return ff_h264_decode_extradata(buf, buf_size, > &h->ps, &h->is_avc, > &h->nal_length_size, > avctx->err_recognition, > avctx); > -- > 2.26.0.110.g2183baf09c-goog > > Would it be possible for someone to take a look at this? It would be great to get this merged. For context, this is needed for us to add an FFmpeg video decoder extension to the Android ExoPlayer project (without having to make the code to do so unnecessarily complicated / inefficient). Thanks!
On Tue, 2 Jun 2020 at 22:32, Olly Woodman <ollywoodman@gmail.com> wrote: > > > On Fri, 8 May 2020 at 03:10, Oliver Woodman <ollywoodman@gmail.com> wrote: > >> https://github.com/FFmpeg/FFmpeg/commit/601c238 added support >> for AV_PKT_DATA_NEW_EXTRADATA, but only for avcC extradata. >> This commit adds support for sps/pps extradata as well. This >> makes support consistent for passing new extradata consistent >> with the types of extradata that can be passed when initializing >> the decoder. >> >> Signed-off-by: Oliver Woodman <ollywoodman@gmail.com> >> --- >> libavcodec/h264dec.c | 13 ++++++------- >> 1 file changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c >> index 4c355feb18..c1d103a474 100644 >> --- a/libavcodec/h264dec.c >> +++ b/libavcodec/h264dec.c >> @@ -829,7 +829,7 @@ static int output_frame(H264Context *h, AVFrame *dst, >> H264Picture *srcp) >> return 0; >> } >> >> -static int is_extra(const uint8_t *buf, int buf_size) >> +static int is_avcc_extradata(const uint8_t *buf, int buf_size) >> { >> int cnt= buf[5]&0x1f; >> const uint8_t *p= buf+6; >> @@ -956,16 +956,15 @@ static int h264_decode_frame(AVCodecContext *avctx, >> void *data, >> if (buf_size == 0) >> return send_next_delayed_frame(h, pict, got_frame, 0); >> >> - if (h->is_avc && av_packet_get_side_data(avpkt, >> AV_PKT_DATA_NEW_EXTRADATA, NULL)) { >> + if (av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA, NULL)) >> { >> int side_size; >> uint8_t *side = av_packet_get_side_data(avpkt, >> AV_PKT_DATA_NEW_EXTRADATA, &side_size); >> - if (is_extra(side, side_size)) >> - ff_h264_decode_extradata(side, side_size, >> - &h->ps, &h->is_avc, >> &h->nal_length_size, >> - avctx->err_recognition, avctx); >> + ff_h264_decode_extradata(side, side_size, >> + &h->ps, &h->is_avc, &h->nal_length_size, >> + avctx->err_recognition, avctx); >> } >> if (h->is_avc && buf_size >= 9 && buf[0]==1 && buf[2]==0 && >> (buf[4]&0xFC)==0xFC) { >> - if (is_extra(buf, buf_size)) >> + if (is_avcc_extradata(buf, buf_size)) >> return ff_h264_decode_extradata(buf, buf_size, >> &h->ps, &h->is_avc, >> &h->nal_length_size, >> avctx->err_recognition, >> avctx); >> -- >> 2.26.0.110.g2183baf09c-goog >> >> > Would it be possible for someone to take a look at this? It would be great > to get this merged. For context, this is needed for us to add an FFmpeg > video decoder extension to the Android ExoPlayer project (without having to > make the code to do so unnecessarily complicated / inefficient). Thanks! > > What do I have to do to get someone to look at (and/or merge) this? Thanks.
On 6/19/20, Olly Woodman <ollywoodman@gmail.com> wrote: > On Tue, 2 Jun 2020 at 22:32, Olly Woodman <ollywoodman@gmail.com> wrote: > >> >> >> On Fri, 8 May 2020 at 03:10, Oliver Woodman <ollywoodman@gmail.com> wrote: >> >>> https://github.com/FFmpeg/FFmpeg/commit/601c238 added support >>> for AV_PKT_DATA_NEW_EXTRADATA, but only for avcC extradata. >>> This commit adds support for sps/pps extradata as well. This >>> makes support consistent for passing new extradata consistent >>> with the types of extradata that can be passed when initializing >>> the decoder. >>> >>> Signed-off-by: Oliver Woodman <ollywoodman@gmail.com> >>> --- >>> libavcodec/h264dec.c | 13 ++++++------- >>> 1 file changed, 6 insertions(+), 7 deletions(-) >>> >>> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c >>> index 4c355feb18..c1d103a474 100644 >>> --- a/libavcodec/h264dec.c >>> +++ b/libavcodec/h264dec.c >>> @@ -829,7 +829,7 @@ static int output_frame(H264Context *h, AVFrame *dst, >>> H264Picture *srcp) >>> return 0; >>> } >>> >>> -static int is_extra(const uint8_t *buf, int buf_size) >>> +static int is_avcc_extradata(const uint8_t *buf, int buf_size) >>> { >>> int cnt= buf[5]&0x1f; >>> const uint8_t *p= buf+6; >>> @@ -956,16 +956,15 @@ static int h264_decode_frame(AVCodecContext *avctx, >>> void *data, >>> if (buf_size == 0) >>> return send_next_delayed_frame(h, pict, got_frame, 0); >>> >>> - if (h->is_avc && av_packet_get_side_data(avpkt, >>> AV_PKT_DATA_NEW_EXTRADATA, NULL)) { >>> + if (av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA, NULL)) >>> { >>> int side_size; >>> uint8_t *side = av_packet_get_side_data(avpkt, >>> AV_PKT_DATA_NEW_EXTRADATA, &side_size); >>> - if (is_extra(side, side_size)) >>> - ff_h264_decode_extradata(side, side_size, >>> - &h->ps, &h->is_avc, >>> &h->nal_length_size, >>> - avctx->err_recognition, avctx); >>> + ff_h264_decode_extradata(side, side_size, >>> + &h->ps, &h->is_avc, >>> &h->nal_length_size, >>> + avctx->err_recognition, avctx); >>> } >>> if (h->is_avc && buf_size >= 9 && buf[0]==1 && buf[2]==0 && >>> (buf[4]&0xFC)==0xFC) { >>> - if (is_extra(buf, buf_size)) >>> + if (is_avcc_extradata(buf, buf_size)) >>> return ff_h264_decode_extradata(buf, buf_size, >>> &h->ps, &h->is_avc, >>> &h->nal_length_size, >>> avctx->err_recognition, >>> avctx); >>> -- >>> 2.26.0.110.g2183baf09c-goog >>> >>> >> Would it be possible for someone to take a look at this? It would be great >> to get this merged. For context, this is needed for us to add an FFmpeg >> video decoder extension to the Android ExoPlayer project (without having >> to >> make the code to do so unnecessarily complicated / inefficient). Thanks! >> >> > > What do I have to do to get someone to look at (and/or merge) this? Thanks. I'm not h264 expert so cannot look at it. Sorry.
On 6/19/2020 5:10 PM, Olly Woodman wrote: > On Tue, 2 Jun 2020 at 22:32, Olly Woodman <ollywoodman@gmail.com> wrote: > >> >> >> On Fri, 8 May 2020 at 03:10, Oliver Woodman <ollywoodman@gmail.com> wrote: >> >>> https://github.com/FFmpeg/FFmpeg/commit/601c238 added support >>> for AV_PKT_DATA_NEW_EXTRADATA, but only for avcC extradata. >>> This commit adds support for sps/pps extradata as well. This >>> makes support consistent for passing new extradata consistent >>> with the types of extradata that can be passed when initializing >>> the decoder. >>> >>> Signed-off-by: Oliver Woodman <ollywoodman@gmail.com> >>> --- >>> libavcodec/h264dec.c | 13 ++++++------- >>> 1 file changed, 6 insertions(+), 7 deletions(-) >>> >>> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c >>> index 4c355feb18..c1d103a474 100644 >>> --- a/libavcodec/h264dec.c >>> +++ b/libavcodec/h264dec.c >>> @@ -829,7 +829,7 @@ static int output_frame(H264Context *h, AVFrame *dst, >>> H264Picture *srcp) >>> return 0; >>> } >>> >>> -static int is_extra(const uint8_t *buf, int buf_size) >>> +static int is_avcc_extradata(const uint8_t *buf, int buf_size) >>> { >>> int cnt= buf[5]&0x1f; >>> const uint8_t *p= buf+6; >>> @@ -956,16 +956,15 @@ static int h264_decode_frame(AVCodecContext *avctx, >>> void *data, >>> if (buf_size == 0) >>> return send_next_delayed_frame(h, pict, got_frame, 0); >>> >>> - if (h->is_avc && av_packet_get_side_data(avpkt, >>> AV_PKT_DATA_NEW_EXTRADATA, NULL)) { >>> + if (av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA, NULL)) >>> { >>> int side_size; >>> uint8_t *side = av_packet_get_side_data(avpkt, >>> AV_PKT_DATA_NEW_EXTRADATA, &side_size); >>> - if (is_extra(side, side_size)) >>> - ff_h264_decode_extradata(side, side_size, >>> - &h->ps, &h->is_avc, >>> &h->nal_length_size, >>> - avctx->err_recognition, avctx); >>> + ff_h264_decode_extradata(side, side_size, >>> + &h->ps, &h->is_avc, &h->nal_length_size, >>> + avctx->err_recognition, avctx); >>> } >>> if (h->is_avc && buf_size >= 9 && buf[0]==1 && buf[2]==0 && >>> (buf[4]&0xFC)==0xFC) { >>> - if (is_extra(buf, buf_size)) >>> + if (is_avcc_extradata(buf, buf_size)) >>> return ff_h264_decode_extradata(buf, buf_size, >>> &h->ps, &h->is_avc, >>> &h->nal_length_size, >>> avctx->err_recognition, >>> avctx); >>> -- >>> 2.26.0.110.g2183baf09c-goog >>> >>> >> Would it be possible for someone to take a look at this? It would be great >> to get this merged. For context, this is needed for us to add an FFmpeg >> video decoder extension to the Android ExoPlayer project (without having to >> make the code to do so unnecessarily complicated / inefficient). Thanks! >> >> > > What do I have to do to get someone to look at (and/or merge) this? Thanks. Just applied it as I hadn't seen it till now. Thank you, and sorry.
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c index 4c355feb18..c1d103a474 100644 --- a/libavcodec/h264dec.c +++ b/libavcodec/h264dec.c @@ -829,7 +829,7 @@ static int output_frame(H264Context *h, AVFrame *dst, H264Picture *srcp) return 0; } -static int is_extra(const uint8_t *buf, int buf_size) +static int is_avcc_extradata(const uint8_t *buf, int buf_size) { int cnt= buf[5]&0x1f; const uint8_t *p= buf+6; @@ -956,16 +956,15 @@ static int h264_decode_frame(AVCodecContext *avctx, void *data, if (buf_size == 0) return send_next_delayed_frame(h, pict, got_frame, 0); - if (h->is_avc && av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA, NULL)) { + if (av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA, NULL)) { int side_size; uint8_t *side = av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA, &side_size); - if (is_extra(side, side_size)) - ff_h264_decode_extradata(side, side_size, - &h->ps, &h->is_avc, &h->nal_length_size, - avctx->err_recognition, avctx); + ff_h264_decode_extradata(side, side_size, + &h->ps, &h->is_avc, &h->nal_length_size, + avctx->err_recognition, avctx); } if (h->is_avc && buf_size >= 9 && buf[0]==1 && buf[2]==0 && (buf[4]&0xFC)==0xFC) { - if (is_extra(buf, buf_size)) + if (is_avcc_extradata(buf, buf_size)) return ff_h264_decode_extradata(buf, buf_size, &h->ps, &h->is_avc, &h->nal_length_size, avctx->err_recognition, avctx);
https://github.com/FFmpeg/FFmpeg/commit/601c238 added support for AV_PKT_DATA_NEW_EXTRADATA, but only for avcC extradata. This commit adds support for sps/pps extradata as well. This makes support consistent for passing new extradata consistent with the types of extradata that can be passed when initializing the decoder. Signed-off-by: Oliver Woodman <ollywoodman@gmail.com> --- libavcodec/h264dec.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)