| Message ID | 20210316083019.25686-1-zane@zanevaniperen.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,v2] avformat/pp_bnk: treat music files are stereo | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | fail | Make fate failed |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | warning | Make fate failed |
Zane van Iperen: > These files are technically a series of planar mono tracks. > If the "music" flag is set, merge the packets from the two > mono tracks, essentially replicating: > > [0:a:0][0:a:1]join=inputs=2:channel_layout=stereo[a] > > Signed-off-by: Zane van Iperen <zane@zanevaniperen.com> > --- > libavformat/pp_bnk.c | 51 ++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 45 insertions(+), 6 deletions(-) > > diff --git a/libavformat/pp_bnk.c b/libavformat/pp_bnk.c > index 8364de1fd9..eba53a01a3 100644 > --- a/libavformat/pp_bnk.c > +++ b/libavformat/pp_bnk.c > @@ -55,6 +55,7 @@ typedef struct PPBnkCtx { > int track_count; > PPBnkCtxTrack *tracks; > uint32_t current_track; > + int is_music; > } PPBnkCtx; > > enum { > @@ -194,8 +195,12 @@ static int pp_bnk_read_header(AVFormatContext *s) > goto fail; > } > > + ctx->is_music = (hdr.flags & PP_BNK_FLAG_MUSIC) && > + (ctx->track_count == 2) && > + (ctx->tracks[0].data_size == ctx->tracks[1].data_size); > + > /* Build the streams. */ > - for (int i = 0; i < ctx->track_count; i++) { > + for (int i = 0; i < (ctx->is_music ? 1 : ctx->track_count); i++) { > if (!(st = avformat_new_stream(s, NULL))) { > ret = AVERROR(ENOMEM); > goto fail; > @@ -204,14 +209,21 @@ static int pp_bnk_read_header(AVFormatContext *s) > par = st->codecpar; > par->codec_type = AVMEDIA_TYPE_AUDIO; > par->codec_id = AV_CODEC_ID_ADPCM_IMA_CUNNING; > - par->format = AV_SAMPLE_FMT_S16; > - par->channel_layout = AV_CH_LAYOUT_MONO; > - par->channels = 1; > + par->format = AV_SAMPLE_FMT_S16P; > + > + if (ctx->is_music) { > + par->channel_layout = AV_CH_LAYOUT_STEREO; > + par->channels = 2; > + } else { > + par->channel_layout = AV_CH_LAYOUT_MONO; > + par->channels = 1; > + } > + > par->sample_rate = hdr.sample_rate; > par->bits_per_coded_sample = 4; > par->bits_per_raw_sample = 16; > par->block_align = 1; > - par->bit_rate = par->sample_rate * par->bits_per_coded_sample; > + par->bit_rate = par->sample_rate * par->bits_per_coded_sample * par->channels; > > avpriv_set_pts_info(st, 64, 1, par->sample_rate); > st->start_time = 0; > @@ -253,7 +265,22 @@ static int pp_bnk_read_packet(AVFormatContext *s, AVPacket *pkt) > > size = FFMIN(trk->data_size - trk->bytes_read, PP_BNK_MAX_READ_SIZE); > > - if ((ret = av_get_packet(s->pb, pkt, size)) == AVERROR_EOF) { > + if (!ctx->is_music) > + ret = av_new_packet(pkt, size); > + else if (ctx->current_track == 0) > + ret = av_new_packet(pkt, size * 2); > + else > + ret = 0; > + > + if (ret < 0) > + return ret; > + > + if (ctx->is_music) > + ret = avio_read(s->pb, pkt->data + size * ctx->current_track, size); > + else > + ret = avio_read(s->pb, pkt->data, size); > + > + if (ret == AVERROR_EOF) { > /* If we've hit EOF, don't attempt this track again. */ > trk->data_size = trk->bytes_read; > continue; > @@ -265,6 +292,18 @@ static int pp_bnk_read_packet(AVFormatContext *s, AVPacket *pkt) > pkt->flags &= ~AV_PKT_FLAG_CORRUPT; > pkt->stream_index = ctx->current_track++; > pkt->duration = ret * 2; > + > + if (ctx->is_music) { > + if (pkt->stream_index == 0) { > + ctx->current_track--; I have to admit to be confused by this. Won't this imply that ctx->current_track will always be zero for music files until you hit the bytes_read == data_size check and that you just overwrite and therefore leak the already allocated packets? > + continue; > + } > + > + pkt->stream_index = 0; > + } else { > + pkt->size = ret; > + } > + > return 0; > } > >
On 16/3/21 6:40 pm, Andreas Rheinhardt wrote: >> + >> + if (ctx->is_music) { >> + if (pkt->stream_index == 0) { >> + ctx->current_track--; > > I have to admit to be confused by this. Won't this imply that > ctx->current_track will always be zero for music files until you hit the > bytes_read == data_size check and that you just overwrite and therefore > leak the already allocated packets? > No, because it's a continue instead of a return, I need to counteract the "ctx->current_track++" in the above for(;;).
On 16/3/21 6:47 pm, Zane van Iperen wrote: > > > On 16/3/21 6:40 pm, Andreas Rheinhardt wrote: > >>> + >>> + if (ctx->is_music) { >>> + if (pkt->stream_index == 0) { >>> + ctx->current_track--; >> >> I have to admit to be confused by this. Won't this imply that >> ctx->current_track will always be zero for music files until you hit the >> bytes_read == data_size check and that you just overwrite and therefore >> leak the already allocated packets? >> > > No, because it's a continue instead of a return, I need to counteract the > "ctx->current_track++" in the above for(;;). Forgot to mention, it's also incremented immediately above: pkt->stream_index = ctx->current_track++; ...Yeah, I might clean this up a bit.
Zane van Iperen: > > > On 16/3/21 6:40 pm, Andreas Rheinhardt wrote: > >>> + >>> + if (ctx->is_music) { >>> + if (pkt->stream_index == 0) { >>> + ctx->current_track--; >> >> I have to admit to be confused by this. Won't this imply that >> ctx->current_track will always be zero for music files until you hit the >> bytes_read == data_size check and that you just overwrite and therefore >> leak the already allocated packets? >> > > No, because it's a continue instead of a return, I need to counteract the > "ctx->current_track++" in the above for(;;). I didn't think about that; I only saw pkt->stream_index = ctx->current_track++;. - Andreas
On Tue, Mar 16, 2021 at 18:30:19 +1000, Zane van Iperen wrote:
Nit:
> Subject: avformat/pp_bnk: treat music files are stereo
^ as
Moritz
diff --git a/libavformat/pp_bnk.c b/libavformat/pp_bnk.c index 8364de1fd9..eba53a01a3 100644 --- a/libavformat/pp_bnk.c +++ b/libavformat/pp_bnk.c @@ -55,6 +55,7 @@ typedef struct PPBnkCtx { int track_count; PPBnkCtxTrack *tracks; uint32_t current_track; + int is_music; } PPBnkCtx; enum { @@ -194,8 +195,12 @@ static int pp_bnk_read_header(AVFormatContext *s) goto fail; } + ctx->is_music = (hdr.flags & PP_BNK_FLAG_MUSIC) && + (ctx->track_count == 2) && + (ctx->tracks[0].data_size == ctx->tracks[1].data_size); + /* Build the streams. */ - for (int i = 0; i < ctx->track_count; i++) { + for (int i = 0; i < (ctx->is_music ? 1 : ctx->track_count); i++) { if (!(st = avformat_new_stream(s, NULL))) { ret = AVERROR(ENOMEM); goto fail; @@ -204,14 +209,21 @@ static int pp_bnk_read_header(AVFormatContext *s) par = st->codecpar; par->codec_type = AVMEDIA_TYPE_AUDIO; par->codec_id = AV_CODEC_ID_ADPCM_IMA_CUNNING; - par->format = AV_SAMPLE_FMT_S16; - par->channel_layout = AV_CH_LAYOUT_MONO; - par->channels = 1; + par->format = AV_SAMPLE_FMT_S16P; + + if (ctx->is_music) { + par->channel_layout = AV_CH_LAYOUT_STEREO; + par->channels = 2; + } else { + par->channel_layout = AV_CH_LAYOUT_MONO; + par->channels = 1; + } + par->sample_rate = hdr.sample_rate; par->bits_per_coded_sample = 4; par->bits_per_raw_sample = 16; par->block_align = 1; - par->bit_rate = par->sample_rate * par->bits_per_coded_sample; + par->bit_rate = par->sample_rate * par->bits_per_coded_sample * par->channels; avpriv_set_pts_info(st, 64, 1, par->sample_rate); st->start_time = 0; @@ -253,7 +265,22 @@ static int pp_bnk_read_packet(AVFormatContext *s, AVPacket *pkt) size = FFMIN(trk->data_size - trk->bytes_read, PP_BNK_MAX_READ_SIZE); - if ((ret = av_get_packet(s->pb, pkt, size)) == AVERROR_EOF) { + if (!ctx->is_music) + ret = av_new_packet(pkt, size); + else if (ctx->current_track == 0) + ret = av_new_packet(pkt, size * 2); + else + ret = 0; + + if (ret < 0) + return ret; + + if (ctx->is_music) + ret = avio_read(s->pb, pkt->data + size * ctx->current_track, size); + else + ret = avio_read(s->pb, pkt->data, size); + + if (ret == AVERROR_EOF) { /* If we've hit EOF, don't attempt this track again. */ trk->data_size = trk->bytes_read; continue; @@ -265,6 +292,18 @@ static int pp_bnk_read_packet(AVFormatContext *s, AVPacket *pkt) pkt->flags &= ~AV_PKT_FLAG_CORRUPT; pkt->stream_index = ctx->current_track++; pkt->duration = ret * 2; + + if (ctx->is_music) { + if (pkt->stream_index == 0) { + ctx->current_track--; + continue; + } + + pkt->stream_index = 0; + } else { + pkt->size = ret; + } + return 0; }
These files are technically a series of planar mono tracks. If the "music" flag is set, merge the packets from the two mono tracks, essentially replicating: [0:a:0][0:a:1]join=inputs=2:channel_layout=stereo[a] Signed-off-by: Zane van Iperen <zane@zanevaniperen.com> --- libavformat/pp_bnk.c | 51 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 6 deletions(-)