| Message ID | 20210114162610.1385-1-nicolas.gaullier@cji.paris |
|---|---|
| Headers | show |
| Series | avcodec/dolby_e: Add a parser | expand |
>De : Nicolas Gaullier <nicolas.gaullier@cji.paris> >Envoyé : jeudi 14 janvier 2021 17:26 >À : ffmpeg-devel@ffmpeg.org >Cc : Nicolas Gaullier <nicolas.gaullier@cji.paris> >Objet : [PATCH 0/2] avcodec/dolby_e: Add a parser > >I have limited duplicated code by making the decoder calling the parser. >An option would be to leave all common code to dolby_e.c and move decoding to dolby_edec.c, but that would require either to duplicate 3 "very-internal" functions (skip_input/parse_key/convert_input) 3x times instead of twice >currently, or to share them with ff_ prefix although they are very-low level and difficult to document etc. > >If you have an idea for a better design, please tell me. > >Nicolas Gaullier (2): > avcodec/dolby_e: Add a parser > avcodec/dolby_e: Split decoder/parser files > > Changelog | 1 + > libavcodec/Makefile | 1 + > libavcodec/dolby_e.c | 209 ++++----- > libavcodec/dolby_e.h | 608 +-------------------------- > libavcodec/dolby_e_parser.c | 227 ++++++++++ > libavcodec/dolby_e_parser.h | 41 ++ > libavcodec/dolby_e_parser_internal.h | 46 ++ > libavcodec/dolby_edec.h | 607 ++++++++++++++++++++++++++ > libavcodec/parsers.c | 1 + > libavcodec/version.h | 2 +- > 10 files changed, 1019 insertions(+), 724 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 > >-- >2.27.0.windows.1 I have not received any feedback since my post last week. Could someone find time to start the review ? Thank you very much Nicolas
Changes looks trivial enough. On Thu, Jan 21, 2021 at 1:30 PM Nicolas Gaullier <nicolas.gaullier@cji.paris> wrote: > >De : Nicolas Gaullier <nicolas.gaullier@cji.paris> > >Envoyé : jeudi 14 janvier 2021 17:26 > >À : ffmpeg-devel@ffmpeg.org > >Cc : Nicolas Gaullier <nicolas.gaullier@cji.paris> > >Objet : [PATCH 0/2] avcodec/dolby_e: Add a parser > > > >I have limited duplicated code by making the decoder calling the parser. > >An option would be to leave all common code to dolby_e.c and move > decoding to dolby_edec.c, but that would require either to duplicate 3 > "very-internal" functions (skip_input/parse_key/convert_input) 3x times > instead of twice >currently, or to share them with ff_ prefix although they > are very-low level and difficult to document etc. > > > >If you have an idea for a better design, please tell me. > > > >Nicolas Gaullier (2): > > avcodec/dolby_e: Add a parser > > avcodec/dolby_e: Split decoder/parser files > > > > Changelog | 1 + > > libavcodec/Makefile | 1 + > > libavcodec/dolby_e.c | 209 ++++----- > > libavcodec/dolby_e.h | 608 +-------------------------- > > libavcodec/dolby_e_parser.c | 227 ++++++++++ > > libavcodec/dolby_e_parser.h | 41 ++ > > libavcodec/dolby_e_parser_internal.h | 46 ++ > > libavcodec/dolby_edec.h | 607 ++++++++++++++++++++++++++ > > libavcodec/parsers.c | 1 + > > libavcodec/version.h | 2 +- > > 10 files changed, 1019 insertions(+), 724 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 > > > >-- > >2.27.0.windows.1 > > I have not received any feedback since my post last week. > Could someone find time to start the review ? > Thank you very much > Nicolas > _______________________________________________ > 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".
>De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Paul B Mahol >Envoyé : jeudi 21 janvier 2021 13:36 >À : FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> >Objet : Re: [FFmpeg-devel] [PATCH 0/2] avcodec/dolby_e: Add a parser > >Changes looks trivial enough. If no one object, the two patches may be applied now ? I can rebase to make it straightforward. Thank you, Nicolas
On Mon, Jan 25, 2021 at 12:04 PM Nicolas Gaullier <nicolas.gaullier@cji.paris> wrote: > >De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Paul B > Mahol > >Envoyé : jeudi 21 janvier 2021 13:36 > >À : FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > >Objet : Re: [FFmpeg-devel] [PATCH 0/2] avcodec/dolby_e: Add a parser > > > >Changes looks trivial enough. > > If no one object, the two patches may be applied now ? I can rebase to > make it straightforward. > Please do. > Thank you, > Nicolas > _______________________________________________ > 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".
Jan 25, 2021, 12:03 by nicolas.gaullier@cji.paris: > >De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Paul B Mahol > >Envoyé : jeudi 21 janvier 2021 13:36 > >À : FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > >Objet : Re: [FFmpeg-devel] [PATCH 0/2] avcodec/dolby_e: Add a parser > >> >> > >Changes looks trivial enough. > > If no one object, the two patches may be applied now ? I can rebase to make it straightforward. > Thank you, > Nicolas > Any objections to renaming dolby_edec.h to dolby_e_tab? It's what it is.
On 1/25/2021 12:35 PM, Lynne wrote: > Jan 25, 2021, 12:03 by nicolas.gaullier@cji.paris: > >>> De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Paul B Mahol >>> Envoyé : jeudi 21 janvier 2021 13:36 >>> À : FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> >>> Objet : Re: [FFmpeg-devel] [PATCH 0/2] avcodec/dolby_e: Add a parser >> >>> >>> >>> Changes looks trivial enough. >> >> If no one object, the two patches may be applied now ? I can rebase to make it straightforward. >> Thank you, >> Nicolas >> > > Any objections to renaming dolby_edec.h to dolby_e_tab? > It's what it is. Look at my patch if you can. I moved them to the decoder header. They could even be moved to dolby_e.c since that's the only user of such tables.
Jan 25, 2021, 16:39 by jamrial@gmail.com: > On 1/25/2021 12:35 PM, Lynne wrote: > >> Jan 25, 2021, 12:03 by nicolas.gaullier@cji.paris: >> >>>> De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Paul B Mahol >>>> Envoyé : jeudi 21 janvier 2021 13:36 >>>> À : FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> >>>> Objet : Re: [FFmpeg-devel] [PATCH 0/2] avcodec/dolby_e: Add a parser >>>> >>>> >>>> >>>> Changes looks trivial enough. >>>> >>> >>> If no one object, the two patches may be applied now ? I can rebase to make it straightforward. >>> Thank you, >>> Nicolas >>> >> >> Any objections to renaming dolby_edec.h to dolby_e_tab? >> It's what it is. >> > > Look at my patch if you can. I moved them to the decoder header. > > They could even be moved to dolby_e.c since that's the only user of such tables. > Yeah, I just noticed your patch, was looking through the oldest emails first (tons of emails today on the ML). It's fine. LGTM.