| Message ID | 20220617054451.6127-1-ffmpeg@gyani.pro |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/mfenc: set frame size for audio encoders. | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
On 2022-06-17 11:14 am, Gyan Doshi wrote: > Default avctx->frame_size is 0 which leads to init failure for > audio MediaFoundation encoders. Plan to push tonight. > --- > libavcodec/mfenc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/mfenc.c b/libavcodec/mfenc.c > index 13ed7b3e11..ff9ffe4827 100644 > --- a/libavcodec/mfenc.c > +++ b/libavcodec/mfenc.c > @@ -1066,6 +1066,9 @@ static int mf_init_encoder(AVCodecContext *avctx) > > c->main_subtype = *subtype; > > + if (c->is_audio && !avctx->frame_size) > + avctx->frame_size = 1024; > + > if ((ret = mf_create(avctx, &c->functions, &c->mft, avctx->codec, use_hw)) < 0) > return ret; >
Gyan Doshi: > Default avctx->frame_size is 0 which leads to init failure for > audio MediaFoundation encoders. > --- > libavcodec/mfenc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/mfenc.c b/libavcodec/mfenc.c > index 13ed7b3e11..ff9ffe4827 100644 > --- a/libavcodec/mfenc.c > +++ b/libavcodec/mfenc.c > @@ -1066,6 +1066,9 @@ static int mf_init_encoder(AVCodecContext *avctx) > > c->main_subtype = *subtype; > > + if (c->is_audio && !avctx->frame_size) > + avctx->frame_size = 1024; > + > if ((ret = mf_create(avctx, &c->functions, &c->mft, avctx->codec, use_hw)) < 0) > return ret; > 1. You forgot to mention the ticket that this is supposed to fix; you should also add that this a regression and the commit (827d6fe73d2f5472c1c2128eb14fab6a4db29032) that supposedly introduced the regression (if this is indeed true). 2. Where does this number come from (apart from the user-provided error message)? How did it work before said commit? (It seems that frame_size was unset before this commit, but did it somehow just not matter?) - Andreas
On 2022-06-18 12:19 pm, Andreas Rheinhardt wrote: > Gyan Doshi: >> Default avctx->frame_size is 0 which leads to init failure for >> audio MediaFoundation encoders. >> --- >> libavcodec/mfenc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/libavcodec/mfenc.c b/libavcodec/mfenc.c >> index 13ed7b3e11..ff9ffe4827 100644 >> --- a/libavcodec/mfenc.c >> +++ b/libavcodec/mfenc.c >> @@ -1066,6 +1066,9 @@ static int mf_init_encoder(AVCodecContext *avctx) >> >> c->main_subtype = *subtype; >> >> + if (c->is_audio && !avctx->frame_size) >> + avctx->frame_size = 1024; >> + >> if ((ret = mf_create(avctx, &c->functions, &c->mft, avctx->codec, use_hw)) < 0) >> return ret; >> > 1. You forgot to mention the ticket that this is supposed to fix; you > should also add that this a regression and the commit > (827d6fe73d2f5472c1c2128eb14fab6a4db29032) that supposedly introduced > the regression (if this is indeed true). I initially became aware due to a user query. Saw ticket later. Will add details. > 2. Where does this number come from (apart from the user-provided error > message)? How did it work before said commit? > (It seems that frame_size was unset before this commit, but did it > somehow just not matter?) The MF encoders accept any number of samples per frame. They reframe as necessary for DSP. I choose 1024 as its the most common frame size emitted (PCM/AAC decoders). Regards, Gyan
Quoting Gyan Doshi (2022-06-18 10:28:38) > > > On 2022-06-18 12:19 pm, Andreas Rheinhardt wrote: > > Gyan Doshi: > >> Default avctx->frame_size is 0 which leads to init failure for > >> audio MediaFoundation encoders. > >> --- > >> libavcodec/mfenc.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/libavcodec/mfenc.c b/libavcodec/mfenc.c > >> index 13ed7b3e11..ff9ffe4827 100644 > >> --- a/libavcodec/mfenc.c > >> +++ b/libavcodec/mfenc.c > >> @@ -1066,6 +1066,9 @@ static int mf_init_encoder(AVCodecContext *avctx) > >> > >> c->main_subtype = *subtype; > >> > >> + if (c->is_audio && !avctx->frame_size) > >> + avctx->frame_size = 1024; > >> + > >> if ((ret = mf_create(avctx, &c->functions, &c->mft, avctx->codec, use_hw)) < 0) > >> return ret; > >> > > 1. You forgot to mention the ticket that this is supposed to fix; you > > should also add that this a regression and the commit > > (827d6fe73d2f5472c1c2128eb14fab6a4db29032) that supposedly introduced > > the regression (if this is indeed true). > I initially became aware due to a user query. Saw ticket later. Will > add details. > > > 2. Where does this number come from (apart from the user-provided error > > message)? How did it work before said commit? > > (It seems that frame_size was unset before this commit, but did it > > somehow just not matter?) > > The MF encoders accept any number of samples per frame. They reframe as > necessary for DSP. So set AV_CODEC_CAP_VARIABLE_FRAME_SIZE instead? Or does it have to be constant?
diff --git a/libavcodec/mfenc.c b/libavcodec/mfenc.c index 13ed7b3e11..ff9ffe4827 100644 --- a/libavcodec/mfenc.c +++ b/libavcodec/mfenc.c @@ -1066,6 +1066,9 @@ static int mf_init_encoder(AVCodecContext *avctx) c->main_subtype = *subtype; + if (c->is_audio && !avctx->frame_size) + avctx->frame_size = 1024; + if ((ret = mf_create(avctx, &c->functions, &c->mft, avctx->codec, use_hw)) < 0) return ret;