tis 2023-12-26 klockan 23:30 +0100 skrev Michael Niedermayer: > Hi > > On Tue, Dec 26, 2023 at 04:52:47PM +0100, Tomas Härdin wrote: > > > > [...] > > + > > +static int apc_write_header(AVFormatContext *s) > > +{ > > + AVIOContext *pb = s->pb; > > + AVCodecParameters *par; > > + AVStream *st; > > + > > + if (s->nb_streams != 1) { > > + av_log(s, AV_LOG_ERROR, "Must have exactly one stream\n"); > > + return AVERROR(EINVAL); > > + } > > + > > + st = s->streams[0]; > > + par = st->codecpar; > > + > > + if (par->ch_layout.nb_channels <= 0 || par- > > >ch_layout.nb_channels > 2) { > > + av_log(s, AV_LOG_ERROR, "Must be mono or stereo\n"); > > + return AVERROR(EINVAL); > > + } > > + > > + if (par->extradata_size != 0 && par->extradata_size != 8) { > > + av_log(s, AV_LOG_ERROR, > > + "Must have exactly 0 or 8 bytes of extradata, got > > %i\n", > > + par->extradata_size); > > + return AVERROR(EINVAL); > > + } > > + > > + ffio_wfourcc(pb, "CRYO"); > > + ffio_wfourcc(pb, "_APC"); > > + ffio_wfourcc(pb, "1.20"); > > > + avio_wl32(pb, 0); // number or samples > > please add to the comment "updated in apc_write_trailer()"
Sure > > +static int apc_write_packet(AVFormatContext *s, AVPacket *pkt) > > +{ > > + size_t extradata_size = 0; > > > + const uint8_t *extradata = av_packet_get_side_data( > > + pkt, AV_PKT_DATA_NEW_EXTRADATA, &extradata_size); > > + > > + if (extradata_size == 8) { > > + // we got predictors from encoder > > + // try to seek back end write them > > + int64_t pos = avio_tell(s->pb), err; > > + if ((err = avio_seek(s->pb, 20, SEEK_SET)) >= 0) { > > + avio_write(s->pb, extradata, extradata_size); > > + avio_seek(s->pb, pos, SEEK_SET); > > + } else { > > + av_log(s, AV_LOG_ERROR, "Got predictors from encoder > > but couldn't seek back to write them\n"); > > + // fail since we should always be able to do this > > within the avio cache > > + // unless the encoder gave us predictors way too late > > for some reason > > + return err; > > + } > > + } > > I think the encoder should buffer data or use 2 passes > if it needs future data. seeking back in the muxer is a bit odd. > > if it becomes available always in teh first packet then maybe the > whole header could be written in the first packet Good idea. IIRC the issue was that extradata is either given immediately (remuxing) or after a single-packet delay (encoding), hence the check for *new* metadata > I would suggest, you add APC support to nut. > Thats a good test to ensure the packet and extradata is set at the > right time > for a generic muxer How would I go about doing that? > > +static int apc_write_trailer(AVFormatContext *s) > > +{ > > + int64_t file_size = avio_tell(s->pb); > > + > > + // write length, if we're able to seek back > > + if (avio_seek(s->pb, 12, SEEK_SET) >= 0) { > > > + if (file_size - APC_HEADER_SIZE > > > + UINT32_MAX * s->streams[0]->codecpar- > > >ch_layout.nb_channels / 2) { > > + av_log(s, AV_LOG_ERROR, "File too large\n"); > > + return AVERROR(EINVAL); > > + } > > + > > + avio_wl32(s->pb, (file_size - APC_HEADER_SIZE) * > > + 2 / s->streams[0]->codecpar- > > >ch_layout.nb_channels); > > I think something like this is slightly cleaner: > > > int64_t number_of_samples = (file_size - APC_HEADER_SIZE) * > 2 / s->streams[0]->codecpar- > >ch_layout.nb_channels; This will UB when file_size is a bit above INT64_MAX / 2. But yeah it can probably be made prettier. I'll have a think about it /Tomas _______________________________________________ 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".