Re: [FFmpeg-devel] [PATCH 3/5] avformat/lc3: Add file format for LC3/LC3plus transport

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Antoine Soulier via ffmpeg-devel:
> A file format is described in Bluetooth SIG LC3 and ETSI TS 103 634, for
> test purpose. This is the format implemented here.
> 
> Signed-off-by: Antoine Soulier <asoulier@google.com>
> Signed-off-by: Antoine SOULIER <asoulier@google.com>
> ---
>  libavformat/Makefile     |   3 +
>  libavformat/allformats.c |   3 +
>  libavformat/lc3dec.c     | 140 +++++++++++++++++++++++++++++++++++++++
>  libavformat/lc3enc.c     | 118 +++++++++++++++++++++++++++++++++
>  4 files changed, 264 insertions(+)
>  create mode 100644 libavformat/lc3dec.c
>  create mode 100644 libavformat/lc3enc.c
> 
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 94a949f555..29a38c1d94 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -332,6 +332,9 @@ OBJS-$(CONFIG_KVAG_DEMUXER)              += kvag.o
>  OBJS-$(CONFIG_KVAG_MUXER)                += kvag.o rawenc.o
>  OBJS-$(CONFIG_LAF_DEMUXER)               += lafdec.o
>  OBJS-$(CONFIG_LATM_MUXER)                += latmenc.o rawenc.o
> +OBJS-$(CONFIG_LC3_DEMUXER)               += lc3dec.o
> +OBJS-$(CONFIG_LC3_MUXER)                 += lc3enc.o
> +OBJS-$(CONFIG_LC3_PLUS_MUXER)            += lc3enc.o
>  OBJS-$(CONFIG_LMLM4_DEMUXER)             += lmlm4.o
>  OBJS-$(CONFIG_LOAS_DEMUXER)              += loasdec.o rawdec.o
>  OBJS-$(CONFIG_LUODAT_DEMUXER)            += luodatdec.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index e15d0fa6d7..551b0f0d7b 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -252,6 +252,9 @@ extern const FFInputFormat  ff_kvag_demuxer;
>  extern const FFOutputFormat ff_kvag_muxer;
>  extern const FFInputFormat  ff_laf_demuxer;
>  extern const FFOutputFormat ff_latm_muxer;
> +extern const FFInputFormat  ff_lc3_demuxer;
> +extern const FFOutputFormat ff_lc3_muxer;
> +extern const FFOutputFormat ff_lc3_plus_muxer;
>  extern const FFInputFormat  ff_lmlm4_demuxer;
>  extern const FFInputFormat  ff_loas_demuxer;
>  extern const FFInputFormat  ff_luodat_demuxer;
> diff --git a/libavformat/lc3dec.c b/libavformat/lc3dec.c
> new file mode 100644
> index 0000000000..563384f786
> --- /dev/null
> +++ b/libavformat/lc3dec.c
> @@ -0,0 +1,140 @@
> +/*
> + * LC3 demuxer
> + * Copyright (C) 2024  Antoine Soulier <asoulier@google.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
> + */
> +
> +/**
> + * @file
> + * Based on the file format specified by :
> + *
> + * - Bluetooth SIG - Low Complexity Communication Codec Test Suite
> + *   https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=502301
> + *   3.2.8.2 Reference LC3 Codec Bitstream Format
> + *
> + * - ETSI TI 103 634 V1.4.1 - Low Complexity Communication Codec plus
> + *   https://www.etsi.org/deliver/etsi_ts/103600_103699/103634/01.04.01_60/ts_103634v010401p.pdf
> + *   LC3plus conformance script package
> + */
> +
> +#include <lc3.h>
> +
> +#include "libavcodec/avcodec.h"

What is that needed for? (De)muxers have almost no reason to ever need this.

> +#include "libavcodec/packet.h"
> +#include "libavutil/intreadwrite.h"
> +
> +#include "avformat.h"
> +#include "avio.h"
> +#include "demux.h"
> +#include "internal.h"
> +
> +typedef struct LC3DemuxContext {
> +    int frame_samples;
> +    int64_t position;
> +    int64_t length;
> +} LC3DemuxContext;
> +
> +static int lc3_read_header(AVFormatContext *s)
> +{
> +    LC3DemuxContext *lc3 = s->priv_data;
> +    AVStream *st = NULL;
> +    uint16_t tag, hdr_size;
> +    uint16_t frame_us;
> +    uint32_t length;
> +    bool ep_mode, hr_mode;
> +    int srate_hz, channels, bit_rate;
> +    int num_extra_params, ret;
> +
> +    tag = avio_rb16(s->pb);
> +    hdr_size = avio_rl16(s->pb);
> +
> +    if (tag != 0x1ccc || hdr_size < 9 * sizeof(uint16_t))
> +        return AVERROR_INVALIDDATA;
> +
> +    num_extra_params = hdr_size / sizeof(uint16_t) - 9;
> +
> +    srate_hz = avio_rl16(s->pb) * 100;
> +    bit_rate = avio_rl16(s->pb) * 100;
> +    channels = avio_rl16(s->pb);
> +    frame_us = avio_rl16(s->pb) * 10;
> +    ep_mode  = avio_rl16(s->pb) != 0;
> +    length   = avio_rl32(s->pb);
> +    hr_mode  = num_extra_params >= 1 && avio_rl16(s->pb);
> +
> +    st = avformat_new_stream(s, NULL);
> +    if (!st)
> +        return AVERROR(ENOMEM);
> +
> +    avpriv_set_pts_info(st, 32, 1, srate_hz);
> +    st->duration = length;
> +
> +    st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
> +    st->codecpar->codec_id = frame_us <= 5000 || ep_mode || hr_mode ?
> +                             AV_CODEC_ID_LC3_PLUS : AV_CODEC_ID_LC3;
> +    st->codecpar->sample_rate = srate_hz;
> +    st->codecpar->bit_rate = bit_rate;
> +    st->codecpar->ch_layout.nb_channels = channels;
> +
> +    if ((ret = ff_alloc_extradata(st->codecpar, 6)) < 0) {
> +        av_log(s, AV_LOG_ERROR, "Could not allocate extradata.\n");

Pointless log

> +        return ret;
> +    }
> +
> +    AV_WL16(st->codecpar->extradata + 0, frame_us);
> +    AV_WL16(st->codecpar->extradata + 2, ep_mode);
> +    AV_WL16(st->codecpar->extradata + 4, hr_mode);
> +
> +    lc3->frame_samples = lc3_hr_frame_samples(hr_mode, frame_us, srate_hz);
> +    lc3->position = 0;
> +    lc3->length = st->duration +
> +        lc3_hr_delay_samples(hr_mode, frame_us, srate_hz);
> +
> +    return 0;
> +}
> +
> +static int lc3_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    LC3DemuxContext *lc3 = s->priv_data;
> +    AVIOContext *pb = s->pb;
> +    int ret;
> +
> +    if ((ret = av_get_packet(s->pb, pkt, avio_rl16(pb))) < 0)

Can you avoid the "if ((ret = func()) < 0)" style? We had bugs due to
wrongly placed parentheses with it.

> +        return ret;
> +
> +    pkt->duration = lc3->frame_samples;
> +    lc3->position += lc3->frame_samples;
> +    if (lc3->position > lc3->length) {
> +        pkt->duration -= lc3->position - lc3->length;
> +        lc3->position = lc3->length;
> +    }
> +
> +    return 0;
> +}
> +
> +const FFInputFormat ff_lc3_demuxer = {
> +    .p.name         = "lc3",
> +    .p.long_name    = NULL_IF_CONFIG_SMALL(
> +           "LC3 / LC3plus (Low Complexity Communication Codec)"),
> +    .p.extensions   = "lc3",
> +    .p.flags        = AVFMT_GENERIC_INDEX |
> +                      AVFMT_NOTIMESTAMPS  |
> +                      AVFMT_NO_BYTE_SEEK,
> +    .priv_data_size = sizeof(LC3DemuxContext),
> +    .read_header    = lc3_read_header,
> +    .read_packet    = lc3_read_packet,
> +};
> diff --git a/libavformat/lc3enc.c b/libavformat/lc3enc.c
> new file mode 100644
> index 0000000000..e6a9ddb35d
> --- /dev/null
> +++ b/libavformat/lc3enc.c
> @@ -0,0 +1,118 @@
> +/*
> + * LC3 muxer
> + * Copyright (C) 2024  Antoine Soulier <asoulier@google.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
> + */
> +
> +/**
> + * @file
> + * Based on the file format specified by :
> + *
> + * - Bluetooth SIG - Low Complexity Communication Codec Test Suite
> + *   https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=502301
> + *   3.2.8.2 Reference LC3 Codec Bitstream Format
> + *
> + * - ETSI TI 103 634 V1.4.1 - Low Complexity Communication Codec plus
> + *   https://www.etsi.org/deliver/etsi_ts/103600_103699/103634/01.04.01_60/ts_103634v010401p.pdf
> + *   LC3plus conformance script package
> + */
> +
> +#include <lc3.h>

If this header is needed (it doesn't seem to be, unless for bool (we
typically use int for this)), then these muxers can only be compiled if
liblc3 is available.

> +
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/opt.h"

This muxer doesn't have options.

> +
> +#include "avformat.h"
> +#include "avio.h"
> +#include "mux.h"
> +#include "internal.h"
> +
> +static av_cold int lc3_init(AVFormatContext *s)
> +{
> +    if (s->nb_streams != 1) {
> +        av_log(s, AV_LOG_ERROR, "This muxer only supports a single stream.\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    return 0;
> +}
> +
> +static int lc3_write_header(AVFormatContext *s)
> +{
> +    AVStream *st = s->streams[0];
> +    int channels = st->codecpar->ch_layout.nb_channels;
> +    int srate_hz = st->codecpar->sample_rate;
> +    int bit_rate = st->codecpar->bit_rate;
> +    uint16_t frame_us = 10000;
> +    bool ep_mode = false, hr_mode = false;
> +    uint32_t nb_samples = av_rescale_q(
> +        st->duration, st->time_base, (AVRational){ 1, srate_hz });
> +
> +    if (st->codecpar->extradata_size >= 2)
> +        frame_us = AV_RL16(st->codecpar->extradata + 0);
> +    if (st->codecpar->extradata_size >= 4)
> +        ep_mode = AV_RL16(st->codecpar->extradata + 2);
> +    if (st->codecpar->extradata_size >= 6)

There is no reason to treat arbitrary extradata size as valid.
Why should extradata_size == 2 or == 4 even exist?

> +        hr_mode = AV_RL16(st->codecpar->extradata + 4);
> +
> +    avio_wb16(s->pb, 0x1ccc);
> +    avio_wl16(s->pb, (9 + hr_mode) * sizeof(uint16_t));
> +    avio_wl16(s->pb, srate_hz / 100);
> +    avio_wl16(s->pb, bit_rate / 100);
> +    avio_wl16(s->pb, channels);
> +    avio_wl16(s->pb, frame_us / 10);
> +    avio_wl16(s->pb, ep_mode);
> +    avio_wl32(s->pb, nb_samples);
> +    if (hr_mode)
> +        avio_wl16(s->pb, hr_mode);
> +
> +    return 0;
> +}
> +
> +static int lc3_write_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    avio_wl16(s->pb, pkt->size);
> +    avio_write(s->pb, pkt->data, pkt->size);
> +    return 0;
> +}
> +
> +const FFOutputFormat ff_lc3_muxer = {
> +    .p.name        = "lc3",
> +    .p.long_name   = NULL_IF_CONFIG_SMALL(
> +           "LC3 (Low Complexity Communication Codec)"),
> +    .p.extensions  = "lc3",
> +    .p.audio_codec = AV_CODEC_ID_LC3,
> +    .p.video_codec = AV_CODEC_ID_NONE,
> +    .p.flags       = AVFMT_NOTIMESTAMPS,
> +    .init          = lc3_init,
> +    .write_header  = lc3_write_header,
> +    .write_packet  = lc3_write_packet,
> +};
> +
> +const FFOutputFormat ff_lc3_plus_muxer = {
> +    .p.name        = "lc3_plus",
> +    .p.long_name   = NULL_IF_CONFIG_SMALL(
> +           "LC3Plus (Low Complexity Communication Codec plus)"),
> +    .p.extensions  = "lc3",
> +    .p.audio_codec = AV_CODEC_ID_LC3_PLUS,
> +    .p.video_codec = AV_CODEC_ID_NONE,
> +    .p.flags       = AVFMT_NOTIMESTAMPS,
> +    .init          = lc3_init,
> +    .write_header  = lc3_write_header,
> +    .write_packet  = lc3_write_packet,
> +};

I do not see the point of having duplicate muxers. It is common for
muxers to support multiple codecs.
But given that both the codecs and the muxers don't differentiate
between the different codec_ids at all, there does not seem to be a
point of using two different codec ids at all.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".