[FFmpeg-devel] [PATCH v23 04/10] avformat/evc_demuxer: Added demuxer to handle reading EVC video files
James Almer
jamrial at gmail.com
Wed Jun 7 19:37:37 EEST 2023
On 6/7/2023 1:30 PM, Dawid Kozinski/Multimedia (PLT) /SRPOL/Staff
Engineer/Samsung Electronics wrote:
>
>
>
>
>> -----Original Message-----
>> From: Dawid Kozinski/Multimedia (PLT) /SRPOL/Staff Engineer/Samsung
>> Electronics <d.kozinski at samsung.com>
>> Sent: środa, 7 czerwca 2023 17:41
>> To: 'FFmpeg development discussions and patches' <ffmpeg-
>> devel at ffmpeg.org>
>> Subject: RE: [FFmpeg-devel] [PATCH v23 04/10] avformat/evc_demuxer: Added
>> demuxer to handle reading EVC video files
>>
>>
>>
>>
>>
>>> -----Original Message-----
>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>> James Almer
>>> Sent: środa, 7 czerwca 2023 15:40
>>> To: ffmpeg-devel at ffmpeg.org
>>> Subject: Re: [FFmpeg-devel] [PATCH v23 04/10] avformat/evc_demuxer:
>>> Added demuxer to handle reading EVC video files
>>>
>>> On 6/7/2023 10:36 AM, Dawid Kozinski/Multimedia (PLT) /SRPOL/Staff
>>> Engineer/Samsung Electronics wrote:
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>>>> James Almer
>>>>> Sent: poniedziałek, 29 maja 2023 15:08
>>>>> To: ffmpeg-devel at ffmpeg.org
>>>>> Subject: Re: [FFmpeg-devel] [PATCH v23 04/10] avformat/evc_demuxer:
>>>>> Added demuxer to handle reading EVC video files
>>>>>
>>>>> On 5/26/2023 7:31 AM, Dawid Kozinski wrote:
>>>>>> +static int get_temporal_id(const uint8_t *bits, int bits_size) {
>>>>>> + int temporal_id = 0;
>>>>>> + short t = 0;
>>>>>> +
>>>>>> + if (bits_size >= EVC_NALU_HEADER_SIZE) {
>>>>>> + unsigned char *p = (unsigned char *)bits;
>>>>>> + // forbidden_zero_bit
>>>>>> + if ((p[0] & 0x80) != 0)
>>>>>> + return -1;
>>>>>> +
>>>>>> + for (int i = 0; i < EVC_NALU_HEADER_SIZE; i++)
>>>>>> + t = (t << 8) | p[i];
>>>>>
>>>>> I think this can be replaced with AV_RB16.
>>>>>
>>>>>> +
>>>>>> + temporal_id = (t >> 6) & 0x0007;
>>>>>> + }
>>>>>> +
>>>>>> + return temporal_id;
>>>>>> +}
>>>>>
>>>>> [...]
>>>>>
>>>>>> +static uint32_t read_nal_unit_length(const uint8_t *bits, int
>>>>>> +bits_size) {
>>>>>> + uint32_t nalu_len = 0;
>>>>>> +
>>>>>> + if (bits_size >= EVC_NALU_LENGTH_PREFIX_SIZE) {
>>>>>> +
>>>>>> + int t = 0;
>>>>>> + unsigned char *p = (unsigned char *)bits;
>>>>>> +
>>>>>> + for (int i = 0; i < EVC_NALU_LENGTH_PREFIX_SIZE; i++)
>>>>>> + t = (t << 8) | p[i];
>>>>>
>>>>> AV_RB32.
>>>>>
>>>>>> +
>>>>>> + nalu_len = t;
>>>>>> + if (nalu_len == 0) // Invalid bitstream size
>>>>>> + return 0;
>>>>>> + }
>>>>>> +
>>>>>> + return nalu_len;
>>>>>> +}
>>>>>
>>>>> [...]
>>>>>
>>>>>> +static int evc_frame_merge_filter(AVBSFContext *bsf, AVPacket *out) {
>>>>>> + EVCMergeContext *ctx = bsf->priv_data;
>>>>>> + AVPacket *in = ctx->in;
>>>>>> +
>>>>>> + int free_space = 0;
>>>>>> + size_t nalu_size = 0;
>>>>>> + uint8_t *nalu = NULL;
>>>>>> + int au_end_found = 0;
>>>>>> + int err;
>>>>>> +
>>>>>> + err = ff_bsf_get_packet_ref(bsf, in);
>>>>>> + if (err < 0)
>>>>>> + return err;
>>>>>> +
>>>>>> + nalu_size = read_nal_unit_length(in->data,
>>>>> EVC_NALU_LENGTH_PREFIX_SIZE);
>>>>>> + if(nalu_size <= 0) {
>>>>>> + av_packet_unref(in);
>>>>>> + return AVERROR_INVALIDDATA;
>>>>>> + }
>>>>>> +
>>>>>> + nalu = in->data + EVC_NALU_LENGTH_PREFIX_SIZE;
>>>>>> + nalu_size = in->size - EVC_NALU_LENGTH_PREFIX_SIZE;
>>>>>> +
>>>>>> + // NAL unit parsing needed to determine if end of AU was found
>>>>>> + err = parse_nal_unit(nalu, nalu_size, bsf);
>>>>>> + if (err < 0) {
>>>>>> + av_log(bsf, AV_LOG_ERROR, "NAL Unit parsing error\n");
>>>>>> + av_packet_unref(in);
>>>>>> +
>>>>>> + return err;
>>>>>> + }
>>>>>> +
>>>>>> + au_end_found = end_of_access_unit_found(bsf);
>>>>>> +
>>>>>> + free_space = ctx->au_buffer.capacity - ctx->au_buffer.data_size;
>>>>>> + while( free_space < in->size ) {
>>>>>> + ctx->au_buffer.capacity *= 2;
>>>>>> + free_space = ctx->au_buffer.capacity -
>>>>>> + ctx->au_buffer.data_size;
>>>>>> +
>>>>>> + if(free_space >= in->size) {
>>>>>> + ctx->au_buffer.data = av_realloc(ctx->au_buffer.data,
>>>>>> + ctx-
>>>>>> au_buffer.capacity);
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> + memcpy(ctx->au_buffer.data + ctx->au_buffer.data_size,
>>>>>> + in->data,
>>>>>> + in->size);
>>>>>
>>>>> This is including the Annex-B's nal_unit_lenght value into the
>>>>> assembled
>>>> packet. I
>>>>> assume libxevd expects this? Also, is it defined as part of the
>>>>> ISOBMFF encapsulation? Patch 07/10 does not seem to strip it.
>>>>
>>>> Could you expand on this topic?
>>>> What do you mean by "Patch 07/10 does not seem to strip it"?
>>>> Patch 7/10 expands the implementation of the MOV Muxer to support
>>>> the EVC stream.
>>>> The function ff_isom_write_evcc parses the received data, extracts
>>>> NAL units of types SPS, PPS, APS, SEI, and inserts their contents
>>>> into the corresponding elements of the arrays array, which is a
>>>> component of the EVCDecoderConfigurationRecord structure.
>>>> Additionally, the ff_isom_write_evcc function populates the relevant
>>>> fields of the EVCDecoderConfigurationRecord structure based on the
>>>> parsed SPS NAL unit data, and then calls the evcc_write function,
>>>> which writes the EVCDecoderConfigurationRecord to the stream.
>>>>
>>>> It seems that I don't understand what's wrong with our
>>>> implementation ( I mean MOV muxer for EVC) and it seems I need your
>>>> help in this matter. I need an explanation of what's wrong and what
>>>> should be done to fix
>>> the issue.
>>>
>>> I mean the packets that go into ISOBMFF Sample, not the evcc atom. Is
>>> the encapsulation supposed to include the nal_unit_length from Annex-B?
>>
>> The function mov_write_single_packet(s, pkt) receives packets containing the
>> Access Units (AU) of an EVC stream. Each AU consists of a sequence of NAL
>> units, each preceded by a 4-byte prefix that contains information about the
>> length of the NAL unit. The entire AU is then copied to AVIOContext using
>> avio_write(pb, pkt->data, size).
>>
>
> It seems to me that everything is okay. If I am mistaken, just let me know.
> As we have also completed all the fixes following your review this is the last thing blocking us from creating a new patchset.
If the 4 byte prefix goes into ISOBMFF encapsulation then, it's fine.
More information about the ffmpeg-devel
mailing list