Paul B Mahol: > +static int laf_read_header(AVFormatContext *ctx) > +{ > + LAFContext *s = ctx->priv_data; > + AVIOContext *pb = ctx->pb; > + unsigned st_count, mode; > + unsigned sample_rate; > + int64_t duration; > + int codec_id; > + int quality; > + int bpp; > + > + avio_skip(pb, 9); > + if (avio_rb32(pb) != MKBETAG('H','E','A','D')) > + return AVERROR_INVALIDDATA; > + > + quality = avio_r8(pb); > + if (quality > 3) > + return AVERROR_INVALIDDATA; > + mode = avio_r8(pb); > + if (mode > 1) > + return AVERROR_INVALIDDATA; > + st_count = avio_rl32(pb); > + if (st_count == 0 || st_count > 1024)
I don't know whether the limit of 1024 is arbitrary or something from some spec. If it is the latter, you should use a #define for it and also for the size of the StreamParams array in the ctx. If it is the former, you might just use FF_ARRAY_ELEMS(s->p) instead of 1024 here. Or a define, as you prefer. > + return AVERROR_INVALIDDATA; > + > + for (int i = 0; i < st_count; i++) { > + StreamParams *stp = &s->p[i]; > + > + stp->vertical = av_int2float(avio_rl32(pb)); > + stp->horizontal = av_int2float(avio_rl32(pb)); > + stp->lfe = avio_r8(pb); > + if (stp->lfe) { > + stp->layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_MASK(1, > (AV_CH_LOW_FREQUENCY)); > + } else if (stp->vertical == 0.f && > + stp->horizontal == 0.f) { > + stp->layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_MASK(1, > (AV_CH_FRONT_CENTER)); > + } else if (stp->vertical == 0.f && > + stp->horizontal == -30.f) { > + stp->layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_MASK(1, > (AV_CH_FRONT_LEFT)); > + } else if (stp->vertical == 0.f && > + stp->horizontal == 30.f) { > + stp->layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_MASK(1, > (AV_CH_FRONT_RIGHT)); > + } else if (stp->vertical == 0.f && > + stp->horizontal == -110.f) { > + stp->layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_MASK(1, > (AV_CH_SIDE_LEFT)); > + } else if (stp->vertical == 0.f && > + stp->horizontal == 110.f) { > + stp->layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_MASK(1, > (AV_CH_SIDE_RIGHT)); > + } else { > + stp->layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_MONO; > + } > + } > + > + sample_rate = avio_rl32(pb); > + duration = avio_rl64(pb) / st_count; > + switch (quality) { > + case 0: > + codec_id = AV_CODEC_ID_PCM_U8; > + bpp = 1; > + break; > + case 1: > + codec_id = AV_CODEC_ID_PCM_S16LE; > + bpp = 2; > + break; > + case 2: > + codec_id = AV_CODEC_ID_PCM_F32LE; > + bpp = 4; > + break; > + case 3: > + codec_id = AV_CODEC_ID_PCM_S24LE; > + bpp = 3; > + break; > + } > + > + s->index = 0; > + s->stored_index = 0; > + s->bpp = bpp; > + s->data = av_mallocz(st_count * sample_rate * bpp); sample_rate is read via avio_rl32() and therefore the multiplication on the right can overflow (it's performed in 32bits, so this can happen even on 64bit systems). Maybe use av_calloc(sample_rate, st_count * bpp). But you also need to ensure that sample_rate actually fits into an int and that st_count * sample_rate * bpp performed in the avio_read() below also fits into an int, so you should probably just ensure this here. > + if (!s->data) > + return AVERROR(ENOMEM); > + > + for (int st = 0; st < st_count; st++) { > + StreamParams *stp = &s->p[st]; > + LAFStream *lafst; > + AVCodecParameters *par; > + AVStream *st = avformat_new_stream(ctx, NULL); > + if (!st) > + return AVERROR(ENOMEM); > + > + par = st->codecpar; > + par->codec_id = codec_id; > + par->codec_type = AVMEDIA_TYPE_AUDIO; > + par->ch_layout.nb_channels = 1; > + par->ch_layout = stp->layout; > + par->sample_rate = sample_rate; > + st->duration = duration; > + st->priv_data = lafst = av_mallocz(sizeof(LAFStream)); lafst is set-but-unused. And given that you are already imposing a hardcoded limit on the number of streams you could just add an array of 1024 uint8_t to your context. > + if (!st->priv_data) > + return AVERROR(ENOMEM); > + > + avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate); > + } > + > + return 0; > +} > + > +static int laf_read_packet(AVFormatContext *ctx, AVPacket *pkt) > +{ > + AVIOContext *pb = ctx->pb; > + LAFContext *s = ctx->priv_data; > + AVStream *st = ctx->streams[0]; > + LAFStream *lafst = st->priv_data; > + const int bpp = s->bpp; > + int header_len = (ctx->nb_streams / 8) + !!(ctx->nb_streams & 7); (ctx->nb_streams + 7) / 8. > + int64_t pos; > + int ret; > + > +again: > + if (avio_feof(pb)) > + return AVERROR_EOF; > + > + pos = avio_tell(pb); > + > + if (s->index >= ctx->nb_streams) { > + int cur_st = 0, st_count = 0, st_index = 0; > + > + for (int i = 0; i < header_len; i++) { > + uint8_t val = avio_r8(pb); Given that you impose a limit of 1024 for the number of streams, you can actually put an uint8_t [128] on the stack in this loop and read all the values at once. This would allow to remove the outer loop. (If you used an array of uint8_t instead of the st->priv_data for stored, you could also use that array.) > + > + for (int j = 0; j < 8 && cur_st < ctx->nb_streams; j++, > cur_st++) { > + AVStream *st = ctx->streams[st_index]; > + LAFStream *lafst = st->priv_data; > + > + lafst->stored = 0; > + if (val & 1) { > + lafst->stored = 1; > + st_count++; > + } > + val >>= 1; > + st_index++; > + } > + } > + > + s->index = s->stored_index = 0; > + s->nb_stored = st_count; > + if (!st_count) > + return AVERROR_INVALIDDATA; > + ret = avio_read(pb, s->data, st_count * st->codecpar->sample_rate * > bpp); > + if (ret < 0) > + return ret; > + } > + > + st = ctx->streams[s->index]; > + lafst = st->priv_data; > + while (!lafst->stored) { > + s->index++; > + if (s->index >= ctx->nb_streams) > + goto again; > + lafst = ctx->streams[s->index]->priv_data; > + } > + st = ctx->streams[s->index]; > + > + ret = av_new_packet(pkt, st->codecpar->sample_rate * bpp); > + if (ret < 0) > + return ret; > + > + for (int n = 0; n < st->codecpar->sample_rate; n++) > + memcpy(pkt->data + n * bpp, s->data + n * s->nb_stored * bpp + > s->stored_index * bpp, bpp); This looks like something that can easily trigger a timeout. > + > + pkt->stream_index = s->index; > + pkt->pos = pos; If you have data from multiple streams interleaved, then the first stream will get the position from before reading header_len bytes, but all the other streams will get the position from after reading the common data. IMO all packets should get the position of the common data. > + s->index++; > + s->stored_index++; > + > + return ret; return 0 -- it is not really defined what happens in case read_packet callbacks return positive values (it is currently ignored and some demuxers return the size of the packet, but that is a remnant of an earlier API) which could happen if av_new_packet() were changed to allow to return positive values. > +} > + > +static int laf_read_seek(AVFormatContext *ctx, int stream_index, > + int64_t timestamp, int flags) > +{ > + LAFContext *s = ctx->priv_data; > + > + s->stored_index = s->index = 0; > + > + return -1; > +} > + > +const AVInputFormat ff_laf_demuxer = { > + .name = "laf", > + .long_name = NULL_IF_CONFIG_SMALL("LAF (Limitless Audio Format)"), > + .priv_data_size = sizeof(LAFContext), > + .read_probe = laf_probe, > + .read_header = laf_read_header, > + .read_packet = laf_read_packet, > + .read_seek = laf_read_seek, > + .extensions = "laf", > + .flags = AVFMT_GENERIC_INDEX, > +}; _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".