Andreas Rheinhardt: > Marton Balint: >> And rename it to retimeinterleave, use the pcm_rechunk bitstream filter for >> rechunking. >> >> By seperating the two functions we hopefully get cleaner code. >> >> Signed-off-by: Marton Balint <c...@passwd.hu> >> --- >> configure | 2 + >> libavformat/Makefile | 4 +- >> libavformat/gxfenc.c | 29 ++++++++---- >> libavformat/mux.c | 1 - >> libavformat/mxfenc.c | 32 ++++++++++---- >> libavformat/retimeinterleave.c | 51 >> ++++++++++++++++++++++ >> .../{audiointerleave.h => retimeinterleave.h} | 31 ++++++------- >> libavformat/utils.c | 1 - >> 8 files changed, 111 insertions(+), 40 deletions(-) >> create mode 100644 libavformat/retimeinterleave.c >> rename libavformat/{audiointerleave.h => retimeinterleave.h} (57%) > > Why don't you delete audiointerleave.c? (I would have expected to see > more deletions than insertions.) > >> >> diff --git a/configure b/configure >> index 4f285f0074..f5a84c31bd 100755 >> --- a/configure >> +++ b/configure >> @@ -2722,6 +2722,7 @@ fraps_decoder_select="bswapdsp huffman" >> g2m_decoder_deps="zlib" >> g2m_decoder_select="blockdsp idctdsp jpegtables" >> g729_decoder_select="audiodsp" >> +gxf_encoder_select="pcm_rechunk_bsf" >> h261_decoder_select="mpegvideo" >> h261_encoder_select="mpegvideoenc" >> h263_decoder_select="h263_parser h263dsp mpegvideo qpeldsp" >> @@ -2794,6 +2795,7 @@ mv30_decoder_select="aandcttables blockdsp" >> mvha_decoder_deps="zlib" >> mvha_decoder_select="llviddsp" >> mwsc_decoder_deps="zlib" >> +mxf_encoder_select="pcm_rechunk_bsf" >> mxpeg_decoder_select="mjpeg_decoder" >> nellymoser_decoder_select="mdct sinewin" >> nellymoser_encoder_select="audio_frame_queue mdct sinewin" >> diff --git a/libavformat/Makefile b/libavformat/Makefile >> index d4bed3c113..56ca55fbd5 100644 >> --- a/libavformat/Makefile >> +++ b/libavformat/Makefile >> @@ -205,7 +205,7 @@ OBJS-$(CONFIG_GIF_DEMUXER) += gifdec.o >> OBJS-$(CONFIG_GSM_DEMUXER) += gsmdec.o >> OBJS-$(CONFIG_GSM_MUXER) += rawenc.o >> OBJS-$(CONFIG_GXF_DEMUXER) += gxf.o >> -OBJS-$(CONFIG_GXF_MUXER) += gxfenc.o audiointerleave.o >> +OBJS-$(CONFIG_GXF_MUXER) += gxfenc.o retimeinterleave.o >> OBJS-$(CONFIG_G722_DEMUXER) += g722.o rawdec.o >> OBJS-$(CONFIG_G722_MUXER) += rawenc.o >> OBJS-$(CONFIG_G723_1_DEMUXER) += g723_1.o >> @@ -347,7 +347,7 @@ OBJS-$(CONFIG_MUSX_DEMUXER) += musx.o >> OBJS-$(CONFIG_MV_DEMUXER) += mvdec.o >> OBJS-$(CONFIG_MVI_DEMUXER) += mvi.o >> OBJS-$(CONFIG_MXF_DEMUXER) += mxfdec.o mxf.o >> -OBJS-$(CONFIG_MXF_MUXER) += mxfenc.o mxf.o >> audiointerleave.o avc.o >> +OBJS-$(CONFIG_MXF_MUXER) += mxfenc.o mxf.o >> retimeinterleave.o avc.o >> OBJS-$(CONFIG_MXG_DEMUXER) += mxg.o >> OBJS-$(CONFIG_NC_DEMUXER) += ncdec.o >> OBJS-$(CONFIG_NISTSPHERE_DEMUXER) += nistspheredec.o pcm.o >> diff --git a/libavformat/gxfenc.c b/libavformat/gxfenc.c >> index e7536a6a7e..e95ae99cba 100644 >> --- a/libavformat/gxfenc.c >> +++ b/libavformat/gxfenc.c >> @@ -27,7 +27,7 @@ >> #include "avformat.h" >> #include "internal.h" >> #include "gxf.h" >> -#include "audiointerleave.h" >> +#include "retimeinterleave.h" >> >> #define GXF_AUDIO_PACKET_SIZE 65536 >> >> @@ -44,7 +44,7 @@ typedef struct GXFTimecode{ >> } GXFTimecode; >> >> typedef struct GXFStreamContext { >> - AudioInterleaveContext aic; >> + RetimeInterleaveContext aic; >> uint32_t track_type; >> uint32_t sample_size; >> uint32_t sample_rate; >> @@ -813,14 +813,12 @@ static int gxf_write_header(AVFormatContext *s) >> return -1; >> } >> } >> + ff_retime_interleave_init(&sc->aic, st->time_base); >> /* FIXME first 10 audio tracks are 0 to 9 next 22 are A to V */ >> sc->media_info = media_info<<8 | ('0'+tracks[media_info]++); >> sc->order = s->nb_streams - st->index; >> } >> >> - if (ff_audio_interleave_init(s, GXF_samples_per_frame, (AVRational){ 1, >> 48000 }) < 0) >> - return -1; >> - >> if (tcr && vsc) >> gxf_init_timecode(s, &gxf->tc, tcr->value, vsc->fields); >> >> @@ -877,8 +875,6 @@ static void gxf_deinit(AVFormatContext *s) >> { >> GXFContext *gxf = s->priv_data; >> >> - ff_audio_interleave_close(s); >> - >> av_freep(&gxf->flt_entries); >> av_freep(&gxf->map_offsets); >> } >> @@ -1016,8 +1012,22 @@ static int gxf_interleave_packet(AVFormatContext *s, >> AVPacket *out, AVPacket *pk >> { >> if (pkt && s->streams[pkt->stream_index]->codecpar->codec_type == >> AVMEDIA_TYPE_VIDEO) >> pkt->duration = 2; // enforce 2 fields >> - return ff_audio_rechunk_interleave(s, out, pkt, flush, >> - ff_interleave_packet_per_dts, >> gxf_compare_field_nb); >> + return ff_retime_interleave(s, out, pkt, flush, >> + ff_interleave_packet_per_dts, >> gxf_compare_field_nb); >> +} >> + >> +static int gxf_check_bitstream(struct AVFormatContext *s, const AVPacket >> *pkt) >> +{ >> + int ret = 1; >> + AVStream *st = s->streams[pkt->stream_index]; >> + >> + if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { >> + char arg[32]; >> + snprintf(arg, sizeof(arg), "n=%d", GXF_samples_per_frame); >> + ret = ff_stream_add_bitstream_filter(st, "pcm_rechunk", arg); >> + } >> + >> + return ret; > > I wonder whether these bsfs should not really be inserted in > init()/write_header(). After all, you don't analyze the packets at all. > > (What actually happens if a user disables automatic bitstream filtering? > Do the muxers error out or write invalid files? I hope one doesn't run > into an assert.) > >> } >> >> AVOutputFormat ff_gxf_muxer = { >> @@ -1031,5 +1041,6 @@ AVOutputFormat ff_gxf_muxer = { >> .write_packet = gxf_write_packet, >> .write_trailer = gxf_write_trailer, >> .deinit = gxf_deinit, >> + .check_bitstream = gxf_check_bitstream, >> .interleave_packet = gxf_interleave_packet, >> }; >> diff --git a/libavformat/mux.c b/libavformat/mux.c >> index c2b6d4461e..b189450dd7 100644 >> --- a/libavformat/mux.c >> +++ b/libavformat/mux.c >> @@ -37,7 +37,6 @@ >> #include "libavutil/parseutils.h" >> #include "libavutil/time.h" >> #include "riff.h" >> -#include "audiointerleave.h" >> #include "url.h" >> #include <stdarg.h> >> #if CONFIG_NETWORK >> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c >> index bfce531f54..d381821116 100644 >> --- a/libavformat/mxfenc.c >> +++ b/libavformat/mxfenc.c >> @@ -52,7 +52,7 @@ >> #include "libavcodec/h264_ps.h" >> #include "libavcodec/golomb.h" >> #include "libavcodec/internal.h" >> -#include "audiointerleave.h" >> +#include "retimeinterleave.h" >> #include "avformat.h" >> #include "avio_internal.h" >> #include "internal.h" >> @@ -79,7 +79,7 @@ typedef struct MXFIndexEntry { >> } MXFIndexEntry; >> >> typedef struct MXFStreamContext { >> - AudioInterleaveContext aic; >> + RetimeInterleaveContext aic; >> UID track_essence_element_key; >> int index; ///< index in mxf_essence_container_uls table >> const UID *codec_ul; >> @@ -2593,6 +2593,7 @@ static int mxf_write_header(AVFormatContext *s) >> return -1; >> } >> } >> + ff_retime_interleave_init(&sc->aic, av_inv_q(mxf->tc.rate)); >> >> if (sc->index == -1) { >> sc->index = >> mxf_get_essence_container_ul_index(st->codecpar->codec_id); >> @@ -2646,9 +2647,6 @@ static int mxf_write_header(AVFormatContext *s) >> return AVERROR(ENOMEM); >> mxf->timecode_track->index = -1; >> >> - if (ff_audio_interleave_init(s, 0, av_inv_q(mxf->tc.rate)) < 0) >> - return -1; >> - >> return 0; >> } >> >> @@ -3010,8 +3008,6 @@ static void mxf_deinit(AVFormatContext *s) >> { >> MXFContext *mxf = s->priv_data; >> >> - ff_audio_interleave_close(s); >> - >> av_freep(&mxf->index_entries); >> av_freep(&mxf->body_partition_offset); >> if (mxf->timecode_track) { >> @@ -3087,8 +3083,23 @@ static int mxf_compare_timestamps(AVFormatContext *s, >> const AVPacket *next, >> >> static int mxf_interleave(AVFormatContext *s, AVPacket *out, AVPacket *pkt, >> int flush) >> { >> - return ff_audio_rechunk_interleave(s, out, pkt, flush, >> - mxf_interleave_get_packet, >> mxf_compare_timestamps); >> + return ff_retime_interleave(s, out, pkt, flush, >> + mxf_interleave_get_packet, >> mxf_compare_timestamps); >> +} >> + >> +static int mxf_check_bitstream(struct AVFormatContext *s, const AVPacket >> *pkt) >> +{ >> + int ret = 1; >> + AVStream *st = s->streams[pkt->stream_index]; >> + MXFStreamContext *sc = st->priv_data; >> + >> + if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { >> + char arg[32]; >> + snprintf(arg, sizeof(arg), "r=%d/%d", sc->aic.time_base.den, >> sc->aic.time_base.num); >> + ret = ff_stream_add_bitstream_filter(st, "pcm_rechunk", arg); >> + } >> + >> + return ret; >> } >> >> #define MXF_COMMON_OPTIONS \ >> @@ -3171,6 +3182,7 @@ AVOutputFormat ff_mxf_muxer = { >> .deinit = mxf_deinit, >> .flags = AVFMT_NOTIMESTAMPS, >> .interleave_packet = mxf_interleave, >> + .check_bitstream = mxf_check_bitstream, >> .priv_class = &mxf_muxer_class, >> }; >> >> @@ -3187,6 +3199,7 @@ AVOutputFormat ff_mxf_d10_muxer = { >> .deinit = mxf_deinit, >> .flags = AVFMT_NOTIMESTAMPS, >> .interleave_packet = mxf_interleave, >> + .check_bitstream = mxf_check_bitstream, >> .priv_class = &mxf_d10_muxer_class, >> }; >> >> @@ -3204,5 +3217,6 @@ AVOutputFormat ff_mxf_opatom_muxer = { >> .deinit = mxf_deinit, >> .flags = AVFMT_NOTIMESTAMPS, >> .interleave_packet = mxf_interleave, >> + .check_bitstream = mxf_check_bitstream, >> .priv_class = &mxf_opatom_muxer_class, >> }; >> diff --git a/libavformat/retimeinterleave.c b/libavformat/retimeinterleave.c >> new file mode 100644 >> index 0000000000..9f874e3626 >> --- /dev/null >> +++ b/libavformat/retimeinterleave.c >> @@ -0,0 +1,51 @@ >> +/* >> + * Retime Interleaving functions >> + * >> + * Copyright (c) 2009 Baptiste Coudurier <baptiste dot coudurier at gmail >> dot com> >> + * >> + * This file is part of FFmpeg. >> + * >> + * FFmpeg is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU Lesser General Public >> + * License as published by the Free Software Foundation; either >> + * version 2.1 of the License, or (at your option) any later version. >> + * >> + * FFmpeg is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * Lesser General Public License for more details. >> + * >> + * You should have received a copy of the GNU Lesser General Public >> + * License along with FFmpeg; if not, write to the Free Software >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 >> USA >> + */ >> + >> +#include "libavutil/mathematics.h" >> +#include "avformat.h" >> +#include "retimeinterleave.h" >> +#include "internal.h" >> + >> +void ff_retime_interleave_init(RetimeInterleaveContext *aic, AVRational >> time_base) > > aic doesn't fit anymore now that you renamed the struct to > RetimeInterleaveContext. > >> +{ >> + aic->time_base = time_base; > > Can't one avoid using different timebases here? After all, a muxer is > free to set the streams' timebases as it sees fit; you can thereby > simply demand that the user already delivers the packets in the correct > timebase. But I have to admit not to understand the timestamp logic in > mxfenc.c. > (This made me wonder: If an automatically inserted bitstream filter sets > its output timebase differently from the input timebase, then the > packets will arrive at the muxer with a different timebase than > st->time_base. Whose job would it be to convert to the timebase the > muxer wanted?
I see you have answered this in 4/6. I'd say the muxer, because the generic code doesn't even > know whether the stream's timebase has intentionally been set by the > muxer or whether the muxer can already handle any timebase. Furthermore, > it is not really important right now as there is no case where this > happens. Maybe we should document the timebase handling of automatically > inserted bsfs a bit so that we don't rely on this undocumented feature > in libavformat?) > >> +} >> + >> +int ff_retime_interleave(AVFormatContext *s, AVPacket *out, AVPacket *pkt, >> int flush, >> + int (*get_packet)(AVFormatContext *, AVPacket *, >> AVPacket *, int), >> + int (*compare_ts)(AVFormatContext *, const AVPacket >> *, const AVPacket *)) >> +{ >> + int ret; >> + >> + if (pkt) { >> + AVStream *st = s->streams[pkt->stream_index]; >> + RetimeInterleaveContext *aic = st->priv_data; >> + pkt->duration = av_rescale_q(pkt->duration, st->time_base, >> aic->time_base); > > What actually happens if a user uses the mxf muxer with av_write_frame() > and not av_interleaved_write_frame()? This code would not get executed > at all, so that the timing would be completely wrong (unless both > timebases would already coincide), even if the user supplied the packets > in the correct order. > >> + // rewrite pts and dts to be decoded time line position >> + pkt->pts = pkt->dts = aic->dts; >> + aic->dts += pkt->duration; >> + if ((ret = ff_interleave_add_packet(s, pkt, compare_ts)) < 0) >> + return ret; >> + } >> + >> + return get_packet(s, out, NULL, flush); >> +} >> diff --git a/libavformat/audiointerleave.h b/libavformat/retimeinterleave.h >> similarity index 57% >> rename from libavformat/audiointerleave.h >> rename to libavformat/retimeinterleave.h >> index 0933310f4c..de0a7442b0 100644 >> --- a/libavformat/audiointerleave.h >> +++ b/libavformat/retimeinterleave.h >> @@ -20,36 +20,31 @@ >> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 >> USA >> */ >> >> -#ifndef AVFORMAT_AUDIOINTERLEAVE_H >> -#define AVFORMAT_AUDIOINTERLEAVE_H >> +#ifndef AVFORMAT_RETIMEINTERLEAVE_H >> +#define AVFORMAT_RETIMEINTERLEAVE_H >> >> -#include "libavutil/fifo.h" >> #include "avformat.h" >> >> -typedef struct AudioInterleaveContext { >> - AVFifoBuffer *fifo; >> - unsigned fifo_size; ///< size of currently allocated FIFO >> - int64_t n; ///< number of generated packets >> - int64_t nb_samples; ///< number of generated samples >> +typedef struct RetimeInterleaveContext { >> uint64_t dts; ///< current dts >> - int sample_size; ///< size of one sample all channels >> included >> - int samples_per_frame; ///< samples per frame if fixed, 0 >> otherwise >> - AVRational time_base; ///< time base of output audio packets >> -} AudioInterleaveContext; >> + AVRational time_base; ///< time base of output packets >> +} RetimeInterleaveContext; >> >> -int ff_audio_interleave_init(AVFormatContext *s, const int >> samples_per_frame, AVRational time_base); >> -void ff_audio_interleave_close(AVFormatContext *s); >> +/** >> + * Init the retime interleave context >> + */ >> +void ff_retime_interleave_init(RetimeInterleaveContext *aic, AVRational >> time_base); >> >> /** >> - * Rechunk audio PCM packets per AudioInterleaveContext->samples_per_frame >> - * and interleave them correctly. >> - * The first element of AVStream->priv_data must be AudioInterleaveContext >> + * Retime packets per RetimeInterleaveContext->time_base and interleave them >> + * correctly. >> + * The first element of AVStream->priv_data must be RetimeInterleaveContext >> * when using this function. >> * >> * @param get_packet function will output a packet when streams are >> correctly interleaved. >> * @param compare_ts function will compare AVPackets and decide >> interleaving order. >> */ >> -int ff_audio_rechunk_interleave(AVFormatContext *s, AVPacket *out, AVPacket >> *pkt, int flush, >> +int ff_retime_interleave(AVFormatContext *s, AVPacket *out, AVPacket *pkt, >> int flush, >> int (*get_packet)(AVFormatContext *, AVPacket *, >> AVPacket *, int), >> int (*compare_ts)(AVFormatContext *, const AVPacket >> *, const AVPacket *)); >> >> diff --git a/libavformat/utils.c b/libavformat/utils.c >> index eff73252ec..25ac5f9446 100644 >> --- a/libavformat/utils.c >> +++ b/libavformat/utils.c >> @@ -41,7 +41,6 @@ >> #include "libavcodec/internal.h" >> #include "libavcodec/raw.h" >> >> -#include "audiointerleave.h" >> #include "avformat.h" >> #include "avio_internal.h" >> #include "id3v2.h" >> > _______________________________________________ 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".