[FFmpeg-devel] [PATCH v5 0/9] avformat: wav-s337m support + new probe_stream option

Nicolas Gaullier nicolas.gaullier at cji.paris
Mon Nov 9 12:21:27 EET 2020


>Nicolas Gaullier (12020-11-09):
>> > doc/formats.texi            |  3 ++
>> > libavcodec/dolby_e.c        |  1 +
>> > libavformat/avformat.h      |  9 ++++-
>> > libavformat/options_table.h |  1 +
>> > libavformat/s337m.c         | 73 ++++++++++++++++++++++++++++++++-----
>> > libavformat/s337m.h         | 54 +++++++++++++++++++++++++++
>> > libavformat/utils.c         |  2 +
>> > libavformat/version.h       |  2 +-
>> > libavformat/wavdec.c        | 53 ++++++++++++++++++---------
>> > tests/Makefile              |  1 +
>> > tests/fate/audio.mak        |  3 ++
>> > tests/ref/fate/s337m-wav    | 11 ++++++
>> > 12 files changed, 185 insertions(+), 28 deletions(-)  create mode
>> > 100644 libavformat/s337m.h  create mode 100644 
>> > tests/ref/fate/s337m-wav
>> 
>> I think I have not received any feedback for this v5 posted 3 weeks 
>> ago. I can rebase everything if it can help for reviewing (or 
>> applying if you think it is close to be accepted).
>
>A patch that mixes so many different changes has little chances to be accepted as is. You need to separate changes by function. It would also increase the chances of review.
>
>Regards,

Thank you for your response.
I fully understand that but the fact is, everything is bind :
- the first patch has not much interest alone, but yes, this one can be separated and it has already been discussed with Anton and approved on irc (10/06)
- patch 2 -> 8 are about one single thing : s337m support in wav; I could potentially remove patch 7 which affects spdif, but it has already been reviewed and seems no problem at all
- patch 9 : this is the tricky one as it is the most recent update in this patch set and has received few feedbacks

The fact is, as I explained earlier, patch 9 cannot be separated, because users must have the choice to disable dolby_e probing, this is a hard requirement.
I thought patchs 1->8 were ready - this thread began a long long time ago, I received many reviews ! - and patch 9 could be reviewed "alone".
That would be the best "git history", and I was not afraid by time but...
At the end, I understand your point, for sure.
I will wait another week and if the process is still frozen, I will rework everything : I will propose the patch 9 "Add AVOption" alone (ie. not including dolby_e), and wait for its approval before going on with the dolby_e serie.

Nicolas


More information about the ffmpeg-devel mailing list