On 3/5/20, Lynne <d...@lynne.ee> wrote:
> Mar 5, 2020, 11:57 by one...@gmail.com:
>
>> On 3/5/20, Paul B Mahol <one...@gmail.com> wrote:
>>
>>> On 3/5/20, Andreas Rheinhardt <andreas.rheinha...@gmail.com> wrote:
>>>
>>>> Paul B Mahol:
>>>>
>>>>> On 3/5/20, Andreas Rheinhardt <andreas.rheinha...@gmail.com> wrote:
>>>>>
>>>>>> Am 05.03.2020 um 00:05 schrieb James Almer:
>>>>>>
>>>>>>> On 3/4/2020 7:51 PM, Paul B Mahol wrote:
>>>>>>>
>>>>>>>> On 3/4/20, James Almer <jamr...@gmail.com> wrote:
>>>>>>>>
>>>>>>>>> On 3/4/2020 7:26 PM, Paul B Mahol wrote:
>>>>>>>>>
>>>>>>>>>> Signed-off-by: Paul B Mahol <one...@gmail.com>
>>>>>>>>>> ---
>>>>>>>>>>  libavformat/Makefile     |   1 +
>>>>>>>>>>  libavformat/ac4dec.c     | 104
>>>>>>>>>> +++++++++++++++++++++++++++++++++++++++
>>>>>>>>>>  libavformat/allformats.c |   1 +
>>>>>>>>>>  3 files changed, 106 insertions(+)
>>>>>>>>>>  create mode 100644 libavformat/ac4dec.c
>>>>>>>>>>
>>>>>>>>>> diff --git a/libavformat/Makefile b/libavformat/Makefile
>>>>>>>>>> index e0681058a2..b4e8d20e65 100644
>>>>>>>>>> --- a/libavformat/Makefile
>>>>>>>>>> +++ b/libavformat/Makefile
>>>>>>>>>> @@ -70,6 +70,7 @@ OBJS-$(CONFIG_AA_DEMUXER)                +=
>>>>>>>>>> aadec.o
>>>>>>>>>>  OBJS-$(CONFIG_AAC_DEMUXER)               += aacdec.o apetag.o
>>>>>>>>>> img2.o
>>>>>>>>>> rawdec.o
>>>>>>>>>>  OBJS-$(CONFIG_AC3_DEMUXER)               += ac3dec.o rawdec.o
>>>>>>>>>>  OBJS-$(CONFIG_AC3_MUXER)                 += rawenc.o
>>>>>>>>>> +OBJS-$(CONFIG_AC4_DEMUXER)               += ac4dec.o
>>>>>>>>>>  OBJS-$(CONFIG_ACM_DEMUXER)               += acm.o rawdec.o
>>>>>>>>>>  OBJS-$(CONFIG_ACT_DEMUXER)               += act.o
>>>>>>>>>>  OBJS-$(CONFIG_ADF_DEMUXER)               += bintext.o sauce.o
>>>>>>>>>> diff --git a/libavformat/ac4dec.c b/libavformat/ac4dec.c
>>>>>>>>>> new file mode 100644
>>>>>>>>>> index 0000000000..8c6e539409
>>>>>>>>>> --- /dev/null
>>>>>>>>>> +++ b/libavformat/ac4dec.c
>>>>>>>>>> @@ -0,0 +1,104 @@
>>>>>>>>>> +/*
>>>>>>>>>> + * RAW AC-4 demuxer
>>>>>>>>>> + * Copyright (c) 2019 Paul B Mahol
>>>>>>>>>> + *
>>>>>>>>>> + * 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/avassert.h"
>>>>>>>>>> +#include "libavutil/crc.h"
>>>>>>>>>> +#include "avformat.h"
>>>>>>>>>> +#include "rawdec.h"
>>>>>>>>>> +
>>>>>>>>>> +static int ac4_probe(const AVProbeData *p)
>>>>>>>>>> +{
>>>>>>>>>> +    const uint8_t *buf = p->buf;
>>>>>>>>>> +    int left = p->buf_size;
>>>>>>>>>> +    int max_frames = 0;
>>>>>>>>>> +
>>>>>>>>>> +    while (left > 7) {
>>>>>>>>>> +        int size;
>>>>>>>>>> +
>>>>>>>>>> +        if (buf[0] == 0xAC &&
>>>>>>>>>> +            (buf[1] == 0x40 ||
>>>>>>>>>> +             buf[1] == 0x41)) {
>>>>>>>>>> +            size = (buf[2] << 8) | buf[3];
>>>>>>>>>> +            if (size == 0xFFFF)
>>>>>>>>>> +                size = 3 + (buf[4] << 16) | (buf[5] << 8) |
>>>>>>>>>> buf[6];
>>>>>>>>>> +            size += 4;
>>>>>>>>>> +            if (buf[1] == 0x41)
>>>>>>>>>> +                size += 2;
>>>>>>>>>> +            max_frames++;
>>>>>>>>>> +            left -= size;
>>>>>>>>>> +            buf += size;
>>>>>>>>>> +        } else {
>>>>>>>>>> +            break;
>>>>>>>>>> +        }
>>>>>>>>>> +    }
>>>>>>>>>> +
>>>>>>>>>> +    return FFMIN(AVPROBE_SCORE_MAX, max_frames * 7);
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static int ac4_read_header(AVFormatContext *s)
>>>>>>>>>> +{
>>>>>>>>>> +    AVStream *st;
>>>>>>>>>> +
>>>>>>>>>> +    st = avformat_new_stream(s, NULL);
>>>>>>>>>> +    if (!st)
>>>>>>>>>> +        return AVERROR(ENOMEM);
>>>>>>>>>> +
>>>>>>>>>> +    st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
>>>>>>>>>> +    st->codecpar->codec_id   = AV_CODEC_ID_AC4;
>>>>>>>>>> +
>>>>>>>>>> +    return 0;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static int ac4_read_packet(AVFormatContext *s, AVPacket *pkt)
>>>>>>>>>> +{
>>>>>>>>>> +    AVIOContext *pb = s->pb;
>>>>>>>>>> +    int64_t pos;
>>>>>>>>>> +    uint16_t sync;
>>>>>>>>>> +    int ret, size;
>>>>>>>>>> +
>>>>>>>>>> +    if (avio_feof(s->pb))
>>>>>>>>>> +        return AVERROR_EOF;
>>>>>>>>>> +
>>>>>>>>>> +    pos   = avio_tell(s->pb);
>>>>>>>>>> +    sync = avio_rb16(pb);
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> If there are sync codes then it sounds like the proper thing to do
>>>>>>>>> is,
>>>>>>>>> much like with AC3, writing a trivial parser to assemble frames and
>>>>>>>>> then
>>>>>>>>> use ff_raw_audio_read_header() and ff_raw_read_partial_packet()
>>>>>>>>> here
>>>>>>>>> instead of custom functions.
>>>>>>>>>
>>>>>>>>
>>>>>>>> That is over complication for simple parsing like here.
>>>>>>>> Every raw packet have exact frame size set in bitstream.
>>>>>>>>
>>>>>>>
>>>>>>> So does AC3, judging by how its parser assembles frames.
>>>>>>>
>>>>>>> An AVParser will let you resync after a bad seek, read frames in non
>>>>>>> seekable input like a pipe, read frames within badly muxed files,
>>>>>>> simplify the demuxer, etc, and is a matter of just looking for that
>>>>>>> 16bit sync code and assembling a frame. Essentially just
>>>>>>> re-implementing
>>>>>>> what you already did in ac4_probe().
>>>>>>>
>>>>>>
>>>>>> If it is intended that the actual AVPackets should not contain the
>>>>>> sync code and the size field at all, then this should be dropped in
>>>>>> the demuxer here (but then the demuxer actually needs to check for
>>>>>> sync codes and needs to handle resyncing itself). This will also mean
>>>>>> that memcpy of the data can be avoided; which is not so when relying
>>>>>> on a parser as the packet size is not chosen adaptively for the
>>>>>> content.
>>>>>>
>>>>>> The decoder seems to allow both variants: With header and without
>>>>>> header. Is there a packetization that strips this header away? (And
>>>>>> what is actually contained in the last two bytes in case the sync code
>>>>>> is 0xAC41?)
>>>>>>
>>>>>
>>>>> CRC word.
>>>>>
>>>> You seem to intend for there to be two forms of AC-4 packets (at least
>>>> your decoder tries to support both): The one with syncwords, size
>>>> field and CRC stripped away and another one where these elements are
>>>> still present. This immediately brings a few questions:
>>>>
>>>> 1. Can a stripped packet be mistaken for an unstripped one, i.e. is it
>>>> possible for the data after the header to begin with a syncword?
>>>> 2. How reversible is stripping away the header and the CRC? Stripping
>>>> away the CRC is obviously irreversible unless we presume that the CRC
>>>> matches. But is stripping the size field away reversible (it is not
>>>> reversible if one is allowed to use the three byte size field if the
>>>> two byte size field would suffice)?
>>>> 3. Are there any container that use the stripped version?
>>>>
>>>> If the answer to the third question is no, then I don't think we
>>>> should create a new packetization of AC-4 and potentially incur the
>>>> first problem as well as the inability to retain CRCs for codec copy.
>>>>
>>>
>>> Decoder should support both, but I do not remember of stripped files.
>>>
>>
>> Actually mp4 dash files are stripped one.
>>
>
> I think the raw demuxer should strip the CRCs, that's a container-level
> thing IMO.
> The decoder should only support one version of packets.

I think .ts packets have it, so it would need to be stripped from .ts demuxer.
In specification it is explicitly written that decoder should support both.
_______________________________________________
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".

Reply via email to