| Message ID | 20201019081929.1926-1-nicolas.gaullier@cji.paris |
|---|---|
| Headers | show |
| Series | avformat: wav-s337m support + new probe_stream option | expand |
>Updates: >* patch 1 : commit msg amended (anton) >* patch 3 : 'if' line split (tomas) > >Everthing else: unchanged since v4 > >For remembering: the test sample 512.wav can be downloaded here: >https://0x0.st/zdW-.wav > >Nicolas Gaullier (9): > avcodec/dolby_e: set constant frame_size > avformat/s337m: Split read_packet/get_packet > avformat/s337m: Consider container bit resolution > avformat/s337m: New ff_s337m_probe() > avformat/wavdec: s337m support > avformat/wavdec.c: Reindent after last commit > avformat/wavdec: fix s337m/spdif probing beyond data_end > avformat/wavdec: Test s337m > avformat: Add probe_stream option > > 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). NB: I keep an eye on irc (ngaullier). Thx Nicolas
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,
>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