[FFmpeg-devel] [PATCH 2/2] avcodec/dolby_e: Split decoder/parser files

[James Almer]

On 1/25/2021 10:24 AM, James Almer wrote:
> On 1/25/2021 9:11 AM, Nicolas Gaullier wrote:
>> ---
>>   libavcodec/Makefile                  |   1 +
>>   libavcodec/dolby_e.c                 | 149 +------
>>   libavcodec/dolby_e.h                 | 598 +-------------------------
>>   libavcodec/dolby_e_parser.c          | 227 ++++++++++
>>   libavcodec/dolby_e_parser.h          |  41 ++
>>   libavcodec/dolby_e_parser_internal.h |  46 ++
>>   libavcodec/dolby_edec.h              | 607 +++++++++++++++++++++++++++
>>   7 files changed, 932 insertions(+), 737 deletions(-)
>>   create mode 100644 libavcodec/dolby_e_parser.c
>>   create mode 100644 libavcodec/dolby_e_parser.h
>>   create mode 100644 libavcodec/dolby_e_parser_internal.h
>>   create mode 100644 libavcodec/dolby_edec.h
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index d48fecc5fc..633ebc37f8 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -40,6 +40,7 @@ OBJS = 
>> ac3_parser.o                                                     \
>>          
>> d3d11va.o                                                        \
>>          
>> decode.o                                                         \
>>          
>> dirac.o                                                          \
>> +       
>> dolby_e_parser.o                                                 \
> 
> Don't compile it unconditionally.
> 
>>          
>> dv_profile.o                                                     \
>>          
>> encode.o                                                         \
>>          
>> imgconvert.o                                                     \
> 
> [...]
> 
>> diff --git a/libavcodec/dolby_e_parser.h b/libavcodec/dolby_e_parser.h
>> new file mode 100644
>> index 0000000000..2f9a2b2ebb
>> --- /dev/null
>> +++ b/libavcodec/dolby_e_parser.h
>> @@ -0,0 +1,41 @@
>> +/*
>> + * Copyright (C) 2017 foo86
>> + *
>> + * 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
>> + */
>> +
>> +#ifndef AVCODEC_DOLBY_E_PARSER_H
>> +#define AVCODEC_DOLBY_E_PARSER_H
>> +
>> +#include "dolby_e.h"
>> +
>> +typedef struct DBEParseContext {
>> +    ParseContext pc;
>> +    DBEContext dectx;
>> +
>> +    DolbyEHeaderInfo metadata;
>> +} DBEParseContext;
>> +
>> +static const uint8_t nb_programs_tab[MAX_PROG_CONF + 1] = {
>> +    2, 3, 2, 3, 4, 5, 4, 5, 6, 7, 8, 1, 2, 3, 3, 4, 5, 6, 1, 2, 3, 4, 
>> 1, 1
>> +};
>> +
>> +static const uint8_t nb_channels_tab[MAX_PROG_CONF + 1] = {
>> +    8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 6, 6, 6, 6, 6, 6, 6, 4, 4, 4, 4, 
>> 8, 8
>> +};
> 
> This header doesn't seem to be needed at all. Just add the struct and 
> both arrays to dolby_e_parser.c
> 
>> +
>> +#endif
>> diff --git a/libavcodec/dolby_e_parser_internal.h 
>> b/libavcodec/dolby_e_parser_internal.h
>> new file mode 100644
>> index 0000000000..defd1f97d6
>> --- /dev/null
>> +++ b/libavcodec/dolby_e_parser_internal.h
> 
> This name is odd, as it implies the other header is public, which is not.
> Try to follow instead what codecs like h264, hevc, and mlp do. There's 
> the decoder, the parser, and then a separate source file that is shared 
> by both. That way, neither the decoder or parser depend on each other.
> 
> dolby_e.c/h - Decoder only functions, tables and structures. Compiled 
> when the decoder is enabled.
> dolby_e_parser.c - Parser only functions, tables and structures. 
> Compiled when the parser is enabled.
> dolby_e_parse.c/h - Shared functions, tables and structures. Compiled 
> when either decoder or parser are enabled.

I see Paul pushed this set already, so i figured I'd just go ahead and 
implement the above myself. Will send it in a minute.