Message ID | 20170106013717.1192-1-jamrial@gmail.com |
---|---|
State | New |
Headers | show |
On 1/6/17, James Almer <jamrial@gmail.com> wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/atrac3.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/atrac3.c b/libavcodec/atrac3.c > index 256990b..208762d 100644 > --- a/libavcodec/atrac3.c > +++ b/libavcodec/atrac3.c > @@ -771,7 +771,7 @@ static int atrac3_decode_frame(AVCodecContext *avctx, > void *data, > return avctx->block_align; > } > > -static av_cold void atrac3_init_static_data(void) > +static av_cold void atrac3_decode_init_static_data(AVCodec *codec) > { > int i; > > @@ -791,7 +791,6 @@ static av_cold void atrac3_init_static_data(void) > > static av_cold int atrac3_decode_init(AVCodecContext *avctx) > { > - static int static_init_done; > int i, ret; > int version, delay, samples_per_frame, frame_factor; > const uint8_t *edata_ptr = avctx->extradata; > @@ -802,10 +801,6 @@ static av_cold int atrac3_decode_init(AVCodecContext > *avctx) > return AVERROR(EINVAL); > } > > - if (!static_init_done) > - atrac3_init_static_data(); > - static_init_done = 1; > - > /* Take care of the codec-specific extradata. */ > if (avctx->extradata_size == 14) { > /* Parse the extradata, WAV format */ > @@ -932,6 +927,7 @@ AVCodec ff_atrac3_decoder = { > .id = AV_CODEC_ID_ATRAC3, > .priv_data_size = sizeof(ATRAC3Context), > .init = atrac3_decode_init, > + .init_static_data = atrac3_decode_init_static_data, > .close = atrac3_decode_close, > .decode = atrac3_decode_frame, > .capabilities = AV_CODEC_CAP_SUBFRAMES | AV_CODEC_CAP_DR1, > -- > 2.10.2 The issue is that init_static_data is called even when codec is never gonna be used. And it is called for both encoder and decoder. Do we really want such startup speed and memory overhead?
On 1/6/2017 6:26 AM, Paul B Mahol wrote: > On 1/6/17, James Almer <jamrial@gmail.com> wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/atrac3.c | 8 ++------ >> 1 file changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/libavcodec/atrac3.c b/libavcodec/atrac3.c >> index 256990b..208762d 100644 >> --- a/libavcodec/atrac3.c >> +++ b/libavcodec/atrac3.c >> @@ -771,7 +771,7 @@ static int atrac3_decode_frame(AVCodecContext *avctx, >> void *data, >> return avctx->block_align; >> } >> >> -static av_cold void atrac3_init_static_data(void) >> +static av_cold void atrac3_decode_init_static_data(AVCodec *codec) >> { >> int i; >> >> @@ -791,7 +791,6 @@ static av_cold void atrac3_init_static_data(void) >> >> static av_cold int atrac3_decode_init(AVCodecContext *avctx) >> { >> - static int static_init_done; >> int i, ret; >> int version, delay, samples_per_frame, frame_factor; >> const uint8_t *edata_ptr = avctx->extradata; >> @@ -802,10 +801,6 @@ static av_cold int atrac3_decode_init(AVCodecContext >> *avctx) >> return AVERROR(EINVAL); >> } >> >> - if (!static_init_done) >> - atrac3_init_static_data(); >> - static_init_done = 1; >> - >> /* Take care of the codec-specific extradata. */ >> if (avctx->extradata_size == 14) { >> /* Parse the extradata, WAV format */ >> @@ -932,6 +927,7 @@ AVCodec ff_atrac3_decoder = { >> .id = AV_CODEC_ID_ATRAC3, >> .priv_data_size = sizeof(ATRAC3Context), >> .init = atrac3_decode_init, >> + .init_static_data = atrac3_decode_init_static_data, >> .close = atrac3_decode_close, >> .decode = atrac3_decode_frame, >> .capabilities = AV_CODEC_CAP_SUBFRAMES | AV_CODEC_CAP_DR1, >> -- >> 2.10.2 > > The issue is that init_static_data is called even when codec is never > gonna be used. > And it is called for both encoder and decoder. > > Do we really want such startup speed and memory overhead? Supposedly hardcoded tables builds were added to counter that, at least partially. In any case it's not important for me, so this can be dropped if it has no real advantages. I remember more than one email from people loathing global state, so I assumed it would be proper to do it this way.
On 1/6/17, James Almer <jamrial@gmail.com> wrote: > On 1/6/2017 6:26 AM, Paul B Mahol wrote: >> On 1/6/17, James Almer <jamrial@gmail.com> wrote: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavcodec/atrac3.c | 8 ++------ >>> 1 file changed, 2 insertions(+), 6 deletions(-) >>> >>> diff --git a/libavcodec/atrac3.c b/libavcodec/atrac3.c >>> index 256990b..208762d 100644 >>> --- a/libavcodec/atrac3.c >>> +++ b/libavcodec/atrac3.c >>> @@ -771,7 +771,7 @@ static int atrac3_decode_frame(AVCodecContext *avctx, >>> void *data, >>> return avctx->block_align; >>> } >>> >>> -static av_cold void atrac3_init_static_data(void) >>> +static av_cold void atrac3_decode_init_static_data(AVCodec *codec) >>> { >>> int i; >>> >>> @@ -791,7 +791,6 @@ static av_cold void atrac3_init_static_data(void) >>> >>> static av_cold int atrac3_decode_init(AVCodecContext *avctx) >>> { >>> - static int static_init_done; >>> int i, ret; >>> int version, delay, samples_per_frame, frame_factor; >>> const uint8_t *edata_ptr = avctx->extradata; >>> @@ -802,10 +801,6 @@ static av_cold int atrac3_decode_init(AVCodecContext >>> *avctx) >>> return AVERROR(EINVAL); >>> } >>> >>> - if (!static_init_done) >>> - atrac3_init_static_data(); >>> - static_init_done = 1; >>> - >>> /* Take care of the codec-specific extradata. */ >>> if (avctx->extradata_size == 14) { >>> /* Parse the extradata, WAV format */ >>> @@ -932,6 +927,7 @@ AVCodec ff_atrac3_decoder = { >>> .id = AV_CODEC_ID_ATRAC3, >>> .priv_data_size = sizeof(ATRAC3Context), >>> .init = atrac3_decode_init, >>> + .init_static_data = atrac3_decode_init_static_data, >>> .close = atrac3_decode_close, >>> .decode = atrac3_decode_frame, >>> .capabilities = AV_CODEC_CAP_SUBFRAMES | AV_CODEC_CAP_DR1, >>> -- >>> 2.10.2 >> >> The issue is that init_static_data is called even when codec is never >> gonna be used. >> And it is called for both encoder and decoder. >> >> Do we really want such startup speed and memory overhead? > > Supposedly hardcoded tables builds were added to counter that, at least > partially. > > In any case it's not important for me, so this can be dropped if it has > no real advantages. I remember more than one email from people loathing > global state, so I assumed it would be proper to do it this way. I have no real opinion on this, but maybe others do have? We could add smarter init_static_data...
Le septidi 17 nivôse, an CCXXV, Paul B Mahol a écrit : > I have no real opinion on this, but maybe others do have? > > We could add smarter init_static_data... Changing the framework to call init_static_data only the first time the codec is ever opened seems like the obvious thing to do. It requires an atomic flag per codec, I think we can devise a way. By the way, the sine table generator in asrc_sine is significantly faster than the float version from the libc, and bit-exact with itself to boot, maybe you can make use of it. Regards,
On Fri, Jan 06, 2017 at 02:54:42PM +0100, Paul B Mahol wrote: > On 1/6/17, James Almer <jamrial@gmail.com> wrote: > > On 1/6/2017 6:26 AM, Paul B Mahol wrote: > >> On 1/6/17, James Almer <jamrial@gmail.com> wrote: > >>> Signed-off-by: James Almer <jamrial@gmail.com> > >>> --- > >>> libavcodec/atrac3.c | 8 ++------ > >>> 1 file changed, 2 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/libavcodec/atrac3.c b/libavcodec/atrac3.c > >>> index 256990b..208762d 100644 > >>> --- a/libavcodec/atrac3.c > >>> +++ b/libavcodec/atrac3.c > >>> @@ -771,7 +771,7 @@ static int atrac3_decode_frame(AVCodecContext *avctx, > >>> void *data, > >>> return avctx->block_align; > >>> } > >>> > >>> -static av_cold void atrac3_init_static_data(void) > >>> +static av_cold void atrac3_decode_init_static_data(AVCodec *codec) > >>> { > >>> int i; > >>> > >>> @@ -791,7 +791,6 @@ static av_cold void atrac3_init_static_data(void) > >>> > >>> static av_cold int atrac3_decode_init(AVCodecContext *avctx) > >>> { > >>> - static int static_init_done; > >>> int i, ret; > >>> int version, delay, samples_per_frame, frame_factor; > >>> const uint8_t *edata_ptr = avctx->extradata; > >>> @@ -802,10 +801,6 @@ static av_cold int atrac3_decode_init(AVCodecContext > >>> *avctx) > >>> return AVERROR(EINVAL); > >>> } > >>> > >>> - if (!static_init_done) > >>> - atrac3_init_static_data(); > >>> - static_init_done = 1; > >>> - > >>> /* Take care of the codec-specific extradata. */ > >>> if (avctx->extradata_size == 14) { > >>> /* Parse the extradata, WAV format */ > >>> @@ -932,6 +927,7 @@ AVCodec ff_atrac3_decoder = { > >>> .id = AV_CODEC_ID_ATRAC3, > >>> .priv_data_size = sizeof(ATRAC3Context), > >>> .init = atrac3_decode_init, > >>> + .init_static_data = atrac3_decode_init_static_data, > >>> .close = atrac3_decode_close, > >>> .decode = atrac3_decode_frame, > >>> .capabilities = AV_CODEC_CAP_SUBFRAMES | AV_CODEC_CAP_DR1, > >>> -- > >>> 2.10.2 > >> > >> The issue is that init_static_data is called even when codec is never > >> gonna be used. > >> And it is called for both encoder and decoder. > >> > >> Do we really want such startup speed and memory overhead? > > > > Supposedly hardcoded tables builds were added to counter that, at least > > partially. > > > > In any case it's not important for me, so this can be dropped if it has > > no real advantages. I remember more than one email from people loathing > > global state, so I assumed it would be proper to do it this way. > > I have no real opinion on this, but maybe others do have? > > We could add smarter init_static_data... +1 [...]
diff --git a/libavcodec/atrac3.c b/libavcodec/atrac3.c index 256990b..208762d 100644 --- a/libavcodec/atrac3.c +++ b/libavcodec/atrac3.c @@ -771,7 +771,7 @@ static int atrac3_decode_frame(AVCodecContext *avctx, void *data, return avctx->block_align; } -static av_cold void atrac3_init_static_data(void) +static av_cold void atrac3_decode_init_static_data(AVCodec *codec) { int i; @@ -791,7 +791,6 @@ static av_cold void atrac3_init_static_data(void) static av_cold int atrac3_decode_init(AVCodecContext *avctx) { - static int static_init_done; int i, ret; int version, delay, samples_per_frame, frame_factor; const uint8_t *edata_ptr = avctx->extradata; @@ -802,10 +801,6 @@ static av_cold int atrac3_decode_init(AVCodecContext *avctx) return AVERROR(EINVAL); } - if (!static_init_done) - atrac3_init_static_data(); - static_init_done = 1; - /* Take care of the codec-specific extradata. */ if (avctx->extradata_size == 14) { /* Parse the extradata, WAV format */ @@ -932,6 +927,7 @@ AVCodec ff_atrac3_decoder = { .id = AV_CODEC_ID_ATRAC3, .priv_data_size = sizeof(ATRAC3Context), .init = atrac3_decode_init, + .init_static_data = atrac3_decode_init_static_data, .close = atrac3_decode_close, .decode = atrac3_decode_frame, .capabilities = AV_CODEC_CAP_SUBFRAMES | AV_CODEC_CAP_DR1,
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/atrac3.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)