Re: [FFmpeg-devel] [PATCH] mmaldec: Set the output pix_fmt after detecting format
On 21.10.15 19:11, wm4 wrote: On Wed, 21 Oct 2015 18:48:42 +0200 Julian Scheel wrote: Am 21.10.2015 um 17:24 schrieb wm4: On Wed, 21 Oct 2015 17:15:14 +0200 Julian Scheel wrote: Am 21.10.2015 um 16:18 schrieb wm4: On Wed, 21 Oct 2015 16:07:08 +0200 Hendrik Leppkes wrote: On Wed, Oct 21, 2015 at 3:55 PM, Julian Scheel wrote: Wait for the first decoded frame to be returned by mmal before setting pix_fmt. This is important for avformat probing to work properly as it is one of the criterions to decide whether to decode a frame or not for probing. Signed-off-by: Julian Scheel --- libavcodec/mmaldec.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c index 7db90d2..429990a 100644 --- a/libavcodec/mmaldec.c +++ b/libavcodec/mmaldec.c @@ -338,11 +338,6 @@ static av_cold int ffmmal_init_decoder(AVCodecContext *avctx) return AVERROR(ENOSYS); } -if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) -return ret; - -avctx->pix_fmt = ret; - if ((status = mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, &ctx->decoder))) goto fail; @@ -678,6 +673,11 @@ static int ffmmal_read_frame(AVCodecContext *avctx, AVFrame *frame, int *got_fra av_log(avctx, AV_LOG_INFO, "Changing output format.\n"); +if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) +return ret; + +avctx->pix_fmt = ret; + if ((status = mmal_port_disable(decoder->output[0]))) goto done; pix_fmt is already used by the decoder before this point to decide if mmal surfaces or memory buffers are to be used, changing it afterwards will not have the same effect as doing it in init. Oh, you're right. It's used in ffmal_update_format(), so this patch can only work if the decoder sends MMAL_EVENT_FORMAT_CHANGED on init, or if the entire decoding init is delayed and not done in AVCodec.init. I think it would be sufficient to query the pix_fmt via ff_get_format in ffmal_update_format instead of using avctx->pix_fmt. The other paramters should be changeable without disabling the port. But this should be tested probably :) Regarding the question if MMAL_EVENT_FORMAT_CHANGED will be sent by the decoder in any case I have opened an issue for clarification: https://github.com/raspberrypi/firmware/issues/489 ffmal_update_format() is also called in the init function. Isn't this what you're trying to avoid? Well yes, probably we could just skip that call. Can you possibly test if that works without breaking your opaque use case? It's needed for decoder init. Right, a part of it is needed. But I'm pretty sure it'd work for me if decoder init were delayed until the first AVCodec.decode/ffmmal_decode call. (But I'd resist if the first init were to happen any time later - makes it harder to fallback to software decoding if mmal doesn't work.) That should be ok. I should be able to test that tomorrow. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mmaldec: Set the output pix_fmt after detecting format
On Wed, 21 Oct 2015 18:48:42 +0200 Julian Scheel wrote: > Am 21.10.2015 um 17:24 schrieb wm4: > > On Wed, 21 Oct 2015 17:15:14 +0200 > > Julian Scheel wrote: > > > >> Am 21.10.2015 um 16:18 schrieb wm4: > >>> On Wed, 21 Oct 2015 16:07:08 +0200 > >>> Hendrik Leppkes wrote: > >>> > On Wed, Oct 21, 2015 at 3:55 PM, Julian Scheel wrote: > > Wait for the first decoded frame to be returned by mmal before setting > > pix_fmt. This is important for avformat probing to work properly as it > > is one > > of the criterions to decide whether to decode a frame or not for > > probing. > > > > Signed-off-by: Julian Scheel > > --- > >libavcodec/mmaldec.c | 10 +- > >1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c > > index 7db90d2..429990a 100644 > > --- a/libavcodec/mmaldec.c > > +++ b/libavcodec/mmaldec.c > > @@ -338,11 +338,6 @@ static av_cold int > > ffmmal_init_decoder(AVCodecContext *avctx) > >return AVERROR(ENOSYS); > >} > > > > -if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) > > -return ret; > > - > > -avctx->pix_fmt = ret; > > - > >if ((status = > > mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, > > &ctx->decoder))) > >goto fail; > > > > @@ -678,6 +673,11 @@ static int ffmmal_read_frame(AVCodecContext > > *avctx, AVFrame *frame, int *got_fra > > > >av_log(avctx, AV_LOG_INFO, "Changing output format.\n"); > > > > +if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < > > 0) > > +return ret; > > + > > +avctx->pix_fmt = ret; > > + > >if ((status = mmal_port_disable(decoder->output[0]))) > >goto done; > > > > pix_fmt is already used by the decoder before this point to decide if > mmal surfaces or memory buffers are to be used, changing it afterwards > will not have the same effect as doing it in init. > >>> > >>> Oh, you're right. It's used in ffmal_update_format(), so this patch can > >>> only work if the decoder sends MMAL_EVENT_FORMAT_CHANGED on init, or if > >>> the entire decoding init is delayed and not done in AVCodec.init. > >> > >> I think it would be sufficient to query the pix_fmt via ff_get_format in > >> ffmal_update_format instead of using avctx->pix_fmt. The other paramters > >> should be changeable without disabling the port. But this should be > >> tested probably :) > >> Regarding the question if MMAL_EVENT_FORMAT_CHANGED will be sent by the > >> decoder in any case I have opened an issue for clarification: > >> https://github.com/raspberrypi/firmware/issues/489 > > > > ffmal_update_format() is also called in the init function. Isn't this what > > you're trying to avoid? > > Well yes, probably we could just skip that call. Can you possibly test > if that works without breaking your opaque use case? It's needed for decoder init. But I'm pretty sure it'd work for me if decoder init were delayed until the first AVCodec.decode/ffmmal_decode call. (But I'd resist if the first init were to happen any time later - makes it harder to fallback to software decoding if mmal doesn't work.) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mmaldec: Set the output pix_fmt after detecting format
Am 21.10.2015 um 17:24 schrieb wm4: On Wed, 21 Oct 2015 17:15:14 +0200 Julian Scheel wrote: Am 21.10.2015 um 16:18 schrieb wm4: On Wed, 21 Oct 2015 16:07:08 +0200 Hendrik Leppkes wrote: On Wed, Oct 21, 2015 at 3:55 PM, Julian Scheel wrote: Wait for the first decoded frame to be returned by mmal before setting pix_fmt. This is important for avformat probing to work properly as it is one of the criterions to decide whether to decode a frame or not for probing. Signed-off-by: Julian Scheel --- libavcodec/mmaldec.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c index 7db90d2..429990a 100644 --- a/libavcodec/mmaldec.c +++ b/libavcodec/mmaldec.c @@ -338,11 +338,6 @@ static av_cold int ffmmal_init_decoder(AVCodecContext *avctx) return AVERROR(ENOSYS); } -if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) -return ret; - -avctx->pix_fmt = ret; - if ((status = mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, &ctx->decoder))) goto fail; @@ -678,6 +673,11 @@ static int ffmmal_read_frame(AVCodecContext *avctx, AVFrame *frame, int *got_fra av_log(avctx, AV_LOG_INFO, "Changing output format.\n"); +if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) +return ret; + +avctx->pix_fmt = ret; + if ((status = mmal_port_disable(decoder->output[0]))) goto done; pix_fmt is already used by the decoder before this point to decide if mmal surfaces or memory buffers are to be used, changing it afterwards will not have the same effect as doing it in init. Oh, you're right. It's used in ffmal_update_format(), so this patch can only work if the decoder sends MMAL_EVENT_FORMAT_CHANGED on init, or if the entire decoding init is delayed and not done in AVCodec.init. I think it would be sufficient to query the pix_fmt via ff_get_format in ffmal_update_format instead of using avctx->pix_fmt. The other paramters should be changeable without disabling the port. But this should be tested probably :) Regarding the question if MMAL_EVENT_FORMAT_CHANGED will be sent by the decoder in any case I have opened an issue for clarification: https://github.com/raspberrypi/firmware/issues/489 ffmal_update_format() is also called in the init function. Isn't this what you're trying to avoid? Well yes, probably we could just skip that call. Can you possibly test if that works without breaking your opaque use case? -Julian ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mmaldec: Set the output pix_fmt after detecting format
On Wed, 21 Oct 2015 17:15:14 +0200 Julian Scheel wrote: > Am 21.10.2015 um 16:18 schrieb wm4: > > On Wed, 21 Oct 2015 16:07:08 +0200 > > Hendrik Leppkes wrote: > > > >> On Wed, Oct 21, 2015 at 3:55 PM, Julian Scheel wrote: > >>> Wait for the first decoded frame to be returned by mmal before setting > >>> pix_fmt. This is important for avformat probing to work properly as it is > >>> one > >>> of the criterions to decide whether to decode a frame or not for probing. > >>> > >>> Signed-off-by: Julian Scheel > >>> --- > >>> libavcodec/mmaldec.c | 10 +- > >>> 1 file changed, 5 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c > >>> index 7db90d2..429990a 100644 > >>> --- a/libavcodec/mmaldec.c > >>> +++ b/libavcodec/mmaldec.c > >>> @@ -338,11 +338,6 @@ static av_cold int > >>> ffmmal_init_decoder(AVCodecContext *avctx) > >>> return AVERROR(ENOSYS); > >>> } > >>> > >>> -if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) > >>> -return ret; > >>> - > >>> -avctx->pix_fmt = ret; > >>> - > >>> if ((status = > >>> mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, > >>> &ctx->decoder))) > >>> goto fail; > >>> > >>> @@ -678,6 +673,11 @@ static int ffmmal_read_frame(AVCodecContext *avctx, > >>> AVFrame *frame, int *got_fra > >>> > >>> av_log(avctx, AV_LOG_INFO, "Changing output format.\n"); > >>> > >>> +if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) > >>> +return ret; > >>> + > >>> +avctx->pix_fmt = ret; > >>> + > >>> if ((status = mmal_port_disable(decoder->output[0]))) > >>> goto done; > >>> > >> > >> pix_fmt is already used by the decoder before this point to decide if > >> mmal surfaces or memory buffers are to be used, changing it afterwards > >> will not have the same effect as doing it in init. > > > > Oh, you're right. It's used in ffmal_update_format(), so this patch can > > only work if the decoder sends MMAL_EVENT_FORMAT_CHANGED on init, or if > > the entire decoding init is delayed and not done in AVCodec.init. > > I think it would be sufficient to query the pix_fmt via ff_get_format in > ffmal_update_format instead of using avctx->pix_fmt. The other paramters > should be changeable without disabling the port. But this should be > tested probably :) > Regarding the question if MMAL_EVENT_FORMAT_CHANGED will be sent by the > decoder in any case I have opened an issue for clarification: > https://github.com/raspberrypi/firmware/issues/489 ffmal_update_format() is also called in the init function. Isn't this what you're trying to avoid? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mmaldec: Set the output pix_fmt after detecting format
Am 21.10.2015 um 16:18 schrieb wm4: On Wed, 21 Oct 2015 16:07:08 +0200 Hendrik Leppkes wrote: On Wed, Oct 21, 2015 at 3:55 PM, Julian Scheel wrote: Wait for the first decoded frame to be returned by mmal before setting pix_fmt. This is important for avformat probing to work properly as it is one of the criterions to decide whether to decode a frame or not for probing. Signed-off-by: Julian Scheel --- libavcodec/mmaldec.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c index 7db90d2..429990a 100644 --- a/libavcodec/mmaldec.c +++ b/libavcodec/mmaldec.c @@ -338,11 +338,6 @@ static av_cold int ffmmal_init_decoder(AVCodecContext *avctx) return AVERROR(ENOSYS); } -if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) -return ret; - -avctx->pix_fmt = ret; - if ((status = mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, &ctx->decoder))) goto fail; @@ -678,6 +673,11 @@ static int ffmmal_read_frame(AVCodecContext *avctx, AVFrame *frame, int *got_fra av_log(avctx, AV_LOG_INFO, "Changing output format.\n"); +if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) +return ret; + +avctx->pix_fmt = ret; + if ((status = mmal_port_disable(decoder->output[0]))) goto done; pix_fmt is already used by the decoder before this point to decide if mmal surfaces or memory buffers are to be used, changing it afterwards will not have the same effect as doing it in init. Oh, you're right. It's used in ffmal_update_format(), so this patch can only work if the decoder sends MMAL_EVENT_FORMAT_CHANGED on init, or if the entire decoding init is delayed and not done in AVCodec.init. I think it would be sufficient to query the pix_fmt via ff_get_format in ffmal_update_format instead of using avctx->pix_fmt. The other paramters should be changeable without disabling the port. But this should be tested probably :) Regarding the question if MMAL_EVENT_FORMAT_CHANGED will be sent by the decoder in any case I have opened an issue for clarification: https://github.com/raspberrypi/firmware/issues/489 -Julian ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mmaldec: Set the output pix_fmt after detecting format
On Wed, 21 Oct 2015 16:07:08 +0200 Hendrik Leppkes wrote: > On Wed, Oct 21, 2015 at 3:55 PM, Julian Scheel wrote: > > Wait for the first decoded frame to be returned by mmal before setting > > pix_fmt. This is important for avformat probing to work properly as it is > > one > > of the criterions to decide whether to decode a frame or not for probing. > > > > Signed-off-by: Julian Scheel > > --- > > libavcodec/mmaldec.c | 10 +- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c > > index 7db90d2..429990a 100644 > > --- a/libavcodec/mmaldec.c > > +++ b/libavcodec/mmaldec.c > > @@ -338,11 +338,6 @@ static av_cold int ffmmal_init_decoder(AVCodecContext > > *avctx) > > return AVERROR(ENOSYS); > > } > > > > -if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) > > -return ret; > > - > > -avctx->pix_fmt = ret; > > - > > if ((status = > > mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, &ctx->decoder))) > > goto fail; > > > > @@ -678,6 +673,11 @@ static int ffmmal_read_frame(AVCodecContext *avctx, > > AVFrame *frame, int *got_fra > > > > av_log(avctx, AV_LOG_INFO, "Changing output format.\n"); > > > > +if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) > > +return ret; > > + > > +avctx->pix_fmt = ret; > > + > > if ((status = mmal_port_disable(decoder->output[0]))) > > goto done; > > > > pix_fmt is already used by the decoder before this point to decide if > mmal surfaces or memory buffers are to be used, changing it afterwards > will not have the same effect as doing it in init. Oh, you're right. It's used in ffmal_update_format(), so this patch can only work if the decoder sends MMAL_EVENT_FORMAT_CHANGED on init, or if the entire decoding init is delayed and not done in AVCodec.init. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mmaldec: Set the output pix_fmt after detecting format
On Wed, Oct 21, 2015 at 3:55 PM, Julian Scheel wrote: > Wait for the first decoded frame to be returned by mmal before setting > pix_fmt. This is important for avformat probing to work properly as it is one > of the criterions to decide whether to decode a frame or not for probing. > > Signed-off-by: Julian Scheel > --- > libavcodec/mmaldec.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c > index 7db90d2..429990a 100644 > --- a/libavcodec/mmaldec.c > +++ b/libavcodec/mmaldec.c > @@ -338,11 +338,6 @@ static av_cold int ffmmal_init_decoder(AVCodecContext > *avctx) > return AVERROR(ENOSYS); > } > > -if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) > -return ret; > - > -avctx->pix_fmt = ret; > - > if ((status = > mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, &ctx->decoder))) > goto fail; > > @@ -678,6 +673,11 @@ static int ffmmal_read_frame(AVCodecContext *avctx, > AVFrame *frame, int *got_fra > > av_log(avctx, AV_LOG_INFO, "Changing output format.\n"); > > +if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) > +return ret; > + > +avctx->pix_fmt = ret; > + > if ((status = mmal_port_disable(decoder->output[0]))) > goto done; > pix_fmt is already used by the decoder before this point to decide if mmal surfaces or memory buffers are to be used, changing it afterwards will not have the same effect as doing it in init. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mmaldec: Set the output pix_fmt after detecting format
On Wed, 21 Oct 2015 15:55:09 +0200 Julian Scheel wrote: > Wait for the first decoded frame to be returned by mmal before setting > pix_fmt. This is important for avformat probing to work properly as it is one > of the criterions to decide whether to decode a frame or not for probing. > > Signed-off-by: Julian Scheel > --- > libavcodec/mmaldec.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c > index 7db90d2..429990a 100644 > --- a/libavcodec/mmaldec.c > +++ b/libavcodec/mmaldec.c > @@ -338,11 +338,6 @@ static av_cold int ffmmal_init_decoder(AVCodecContext > *avctx) > return AVERROR(ENOSYS); > } > > -if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) > -return ret; > - > -avctx->pix_fmt = ret; > - > if ((status = > mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, &ctx->decoder))) > goto fail; > > @@ -678,6 +673,11 @@ static int ffmmal_read_frame(AVCodecContext *avctx, > AVFrame *frame, int *got_fra > > av_log(avctx, AV_LOG_INFO, "Changing output format.\n"); > > +if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) > +return ret; > + > +avctx->pix_fmt = ret; > + > if ((status = mmal_port_disable(decoder->output[0]))) > goto done; > MMAL_EVENT_FORMAT_CHANGED does not always happen. Some streams start decoding without it. So you'd need to delay this somehow. Maybe when the first decode call is done? (Other places also would work, but to make fallback from hw decoding work well, it should happen when the first packet is being consumed.) I'd also like to test this with opaque formats first. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] mmaldec: Set the output pix_fmt after detecting format
Wait for the first decoded frame to be returned by mmal before setting pix_fmt. This is important for avformat probing to work properly as it is one of the criterions to decide whether to decode a frame or not for probing. Signed-off-by: Julian Scheel --- libavcodec/mmaldec.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c index 7db90d2..429990a 100644 --- a/libavcodec/mmaldec.c +++ b/libavcodec/mmaldec.c @@ -338,11 +338,6 @@ static av_cold int ffmmal_init_decoder(AVCodecContext *avctx) return AVERROR(ENOSYS); } -if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) -return ret; - -avctx->pix_fmt = ret; - if ((status = mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, &ctx->decoder))) goto fail; @@ -678,6 +673,11 @@ static int ffmmal_read_frame(AVCodecContext *avctx, AVFrame *frame, int *got_fra av_log(avctx, AV_LOG_INFO, "Changing output format.\n"); +if ((ret = ff_get_format(avctx, avctx->codec->pix_fmts)) < 0) +return ret; + +avctx->pix_fmt = ret; + if ((status = mmal_port_disable(decoder->output[0]))) goto done; -- 2.6.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
