| Message ID | 20200402171740.19121-2-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | 18d69e9a9829d3846667c0a3f949033c71d2a1a0 |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avformat/ilbc: Don't reimplement ff_raw_write_packet | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Andreas Rheinhardt: > Currently Musepack allocates an array that needs to be freed later in > the demuxer's read_close-function; it is the sole reason for said > function's existence. But it is unnecessary, because one can store this > array in the stream's priv_data pointer, so that it will be freed > generically. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavformat/mpc.c | 40 ++++++++++++++++------------------------ > 1 file changed, 16 insertions(+), 24 deletions(-) > > diff --git a/libavformat/mpc.c b/libavformat/mpc.c > index 85036cd118..6a94b5d1d0 100644 > --- a/libavformat/mpc.c > +++ b/libavformat/mpc.c > @@ -57,6 +57,7 @@ static int mpc_read_header(AVFormatContext *s) > { > MPCContext *c = s->priv_data; > AVStream *st; > + int ret; > > if(avio_rl24(s->pb) != MKTAG('M', 'P', '+', 0)){ > av_log(s, AV_LOG_ERROR, "Not a Musepack file\n"); > @@ -72,15 +73,6 @@ static int mpc_read_header(AVFormatContext *s) > av_log(s, AV_LOG_ERROR, "Too many frames, seeking is not possible\n"); > return AVERROR_INVALIDDATA; > } > - if(c->fcount){ > - c->frames = av_malloc(c->fcount * sizeof(MPCFrame)); > - if(!c->frames){ > - av_log(s, AV_LOG_ERROR, "Cannot allocate seektable\n"); > - return AVERROR(ENOMEM); > - } > - }else{ > - av_log(s, AV_LOG_WARNING, "Container reports no frames\n"); > - } > c->curframe = 0; > c->lastframe = -1; > c->curbits = 8; > @@ -88,15 +80,27 @@ static int mpc_read_header(AVFormatContext *s) > > st = avformat_new_stream(s, NULL); > if (!st) > - goto mem_error; > + return AVERROR(ENOMEM); > + > + if (c->fcount) { > + c->frames = av_malloc(c->fcount * sizeof(MPCFrame)); > + if (!c->frames) { > + av_log(s, AV_LOG_ERROR, "Cannot allocate seektable\n"); > + return AVERROR(ENOMEM); > + } > + st->priv_data = c->frames; > + } else { > + av_log(s, AV_LOG_WARNING, "Container reports no frames\n"); > + } > + > st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; > st->codecpar->codec_id = AV_CODEC_ID_MUSEPACK7; > st->codecpar->channels = 2; > st->codecpar->channel_layout = AV_CH_LAYOUT_STEREO; > st->codecpar->bits_per_coded_sample = 16; > > - if (ff_get_extradata(s, st->codecpar, s->pb, 16) < 0) > - goto mem_error; > + if ((ret = ff_get_extradata(s, st->codecpar, s->pb, 16)) < 0) > + return ret; > st->codecpar->sample_rate = mpc_rate[st->codecpar->extradata[2] & 3]; > avpriv_set_pts_info(st, 32, MPC_FRAMESIZE, st->codecpar->sample_rate); > /* scan for seekpoints */ > @@ -113,9 +117,6 @@ static int mpc_read_header(AVFormatContext *s) > } > > return 0; > -mem_error: > - av_freep(&c->frames); > - return AVERROR(ENOMEM); > } > > static int mpc_read_packet(AVFormatContext *s, AVPacket *pkt) > @@ -176,14 +177,6 @@ static int mpc_read_packet(AVFormatContext *s, AVPacket *pkt) > return 0; > } > > -static int mpc_read_close(AVFormatContext *s) > -{ > - MPCContext *c = s->priv_data; > - > - av_freep(&c->frames); > - return 0; > -} > - > /** > * Seek to the given position > * If position is unknown but is within the limits of file > @@ -232,7 +225,6 @@ AVInputFormat ff_mpc_demuxer = { > .read_probe = mpc_probe, > .read_header = mpc_read_header, > .read_packet = mpc_read_packet, > - .read_close = mpc_read_close, > .read_seek = mpc_read_seek, > .extensions = "mpc", > }; > Will push tomorrow if there are no objections. - Andreas
Andreas Rheinhardt: > Andreas Rheinhardt: >> Currently Musepack allocates an array that needs to be freed later in >> the demuxer's read_close-function; it is the sole reason for said >> function's existence. But it is unnecessary, because one can store this >> array in the stream's priv_data pointer, so that it will be freed >> generically. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >> --- >> libavformat/mpc.c | 40 ++++++++++++++++------------------------ >> 1 file changed, 16 insertions(+), 24 deletions(-) >> >> diff --git a/libavformat/mpc.c b/libavformat/mpc.c >> index 85036cd118..6a94b5d1d0 100644 >> --- a/libavformat/mpc.c >> +++ b/libavformat/mpc.c >> @@ -57,6 +57,7 @@ static int mpc_read_header(AVFormatContext *s) >> { >> MPCContext *c = s->priv_data; >> AVStream *st; >> + int ret; >> >> if(avio_rl24(s->pb) != MKTAG('M', 'P', '+', 0)){ >> av_log(s, AV_LOG_ERROR, "Not a Musepack file\n"); >> @@ -72,15 +73,6 @@ static int mpc_read_header(AVFormatContext *s) >> av_log(s, AV_LOG_ERROR, "Too many frames, seeking is not possible\n"); >> return AVERROR_INVALIDDATA; >> } >> - if(c->fcount){ >> - c->frames = av_malloc(c->fcount * sizeof(MPCFrame)); >> - if(!c->frames){ >> - av_log(s, AV_LOG_ERROR, "Cannot allocate seektable\n"); >> - return AVERROR(ENOMEM); >> - } >> - }else{ >> - av_log(s, AV_LOG_WARNING, "Container reports no frames\n"); >> - } >> c->curframe = 0; >> c->lastframe = -1; >> c->curbits = 8; >> @@ -88,15 +80,27 @@ static int mpc_read_header(AVFormatContext *s) >> >> st = avformat_new_stream(s, NULL); >> if (!st) >> - goto mem_error; >> + return AVERROR(ENOMEM); >> + >> + if (c->fcount) { >> + c->frames = av_malloc(c->fcount * sizeof(MPCFrame)); >> + if (!c->frames) { >> + av_log(s, AV_LOG_ERROR, "Cannot allocate seektable\n"); >> + return AVERROR(ENOMEM); >> + } >> + st->priv_data = c->frames; >> + } else { >> + av_log(s, AV_LOG_WARNING, "Container reports no frames\n"); >> + } >> + >> st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; >> st->codecpar->codec_id = AV_CODEC_ID_MUSEPACK7; >> st->codecpar->channels = 2; >> st->codecpar->channel_layout = AV_CH_LAYOUT_STEREO; >> st->codecpar->bits_per_coded_sample = 16; >> >> - if (ff_get_extradata(s, st->codecpar, s->pb, 16) < 0) >> - goto mem_error; >> + if ((ret = ff_get_extradata(s, st->codecpar, s->pb, 16)) < 0) >> + return ret; >> st->codecpar->sample_rate = mpc_rate[st->codecpar->extradata[2] & 3]; >> avpriv_set_pts_info(st, 32, MPC_FRAMESIZE, st->codecpar->sample_rate); >> /* scan for seekpoints */ >> @@ -113,9 +117,6 @@ static int mpc_read_header(AVFormatContext *s) >> } >> >> return 0; >> -mem_error: >> - av_freep(&c->frames); >> - return AVERROR(ENOMEM); >> } >> >> static int mpc_read_packet(AVFormatContext *s, AVPacket *pkt) >> @@ -176,14 +177,6 @@ static int mpc_read_packet(AVFormatContext *s, AVPacket *pkt) >> return 0; >> } >> >> -static int mpc_read_close(AVFormatContext *s) >> -{ >> - MPCContext *c = s->priv_data; >> - >> - av_freep(&c->frames); >> - return 0; >> -} >> - >> /** >> * Seek to the given position >> * If position is unknown but is within the limits of file >> @@ -232,7 +225,6 @@ AVInputFormat ff_mpc_demuxer = { >> .read_probe = mpc_probe, >> .read_header = mpc_read_header, >> .read_packet = mpc_read_packet, >> - .read_close = mpc_read_close, >> .read_seek = mpc_read_seek, >> .extensions = "mpc", >> }; >> > Will push tomorrow if there are no objections. > > - Andreas > Applied. - Andreas
diff --git a/libavformat/mpc.c b/libavformat/mpc.c index 85036cd118..6a94b5d1d0 100644 --- a/libavformat/mpc.c +++ b/libavformat/mpc.c @@ -57,6 +57,7 @@ static int mpc_read_header(AVFormatContext *s) { MPCContext *c = s->priv_data; AVStream *st; + int ret; if(avio_rl24(s->pb) != MKTAG('M', 'P', '+', 0)){ av_log(s, AV_LOG_ERROR, "Not a Musepack file\n"); @@ -72,15 +73,6 @@ static int mpc_read_header(AVFormatContext *s) av_log(s, AV_LOG_ERROR, "Too many frames, seeking is not possible\n"); return AVERROR_INVALIDDATA; } - if(c->fcount){ - c->frames = av_malloc(c->fcount * sizeof(MPCFrame)); - if(!c->frames){ - av_log(s, AV_LOG_ERROR, "Cannot allocate seektable\n"); - return AVERROR(ENOMEM); - } - }else{ - av_log(s, AV_LOG_WARNING, "Container reports no frames\n"); - } c->curframe = 0; c->lastframe = -1; c->curbits = 8; @@ -88,15 +80,27 @@ static int mpc_read_header(AVFormatContext *s) st = avformat_new_stream(s, NULL); if (!st) - goto mem_error; + return AVERROR(ENOMEM); + + if (c->fcount) { + c->frames = av_malloc(c->fcount * sizeof(MPCFrame)); + if (!c->frames) { + av_log(s, AV_LOG_ERROR, "Cannot allocate seektable\n"); + return AVERROR(ENOMEM); + } + st->priv_data = c->frames; + } else { + av_log(s, AV_LOG_WARNING, "Container reports no frames\n"); + } + st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; st->codecpar->codec_id = AV_CODEC_ID_MUSEPACK7; st->codecpar->channels = 2; st->codecpar->channel_layout = AV_CH_LAYOUT_STEREO; st->codecpar->bits_per_coded_sample = 16; - if (ff_get_extradata(s, st->codecpar, s->pb, 16) < 0) - goto mem_error; + if ((ret = ff_get_extradata(s, st->codecpar, s->pb, 16)) < 0) + return ret; st->codecpar->sample_rate = mpc_rate[st->codecpar->extradata[2] & 3]; avpriv_set_pts_info(st, 32, MPC_FRAMESIZE, st->codecpar->sample_rate); /* scan for seekpoints */ @@ -113,9 +117,6 @@ static int mpc_read_header(AVFormatContext *s) } return 0; -mem_error: - av_freep(&c->frames); - return AVERROR(ENOMEM); } static int mpc_read_packet(AVFormatContext *s, AVPacket *pkt) @@ -176,14 +177,6 @@ static int mpc_read_packet(AVFormatContext *s, AVPacket *pkt) return 0; } -static int mpc_read_close(AVFormatContext *s) -{ - MPCContext *c = s->priv_data; - - av_freep(&c->frames); - return 0; -} - /** * Seek to the given position * If position is unknown but is within the limits of file @@ -232,7 +225,6 @@ AVInputFormat ff_mpc_demuxer = { .read_probe = mpc_probe, .read_header = mpc_read_header, .read_packet = mpc_read_packet, - .read_close = mpc_read_close, .read_seek = mpc_read_seek, .extensions = "mpc", };
Currently Musepack allocates an array that needs to be freed later in the demuxer's read_close-function; it is the sole reason for said function's existence. But it is unnecessary, because one can store this array in the stream's priv_data pointer, so that it will be freed generically. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavformat/mpc.c | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-)