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".
