[FFmpeg-devel] [PATCH v3] avformat/nutdec: Don't create inconsistent side data

[Michael Niedermayer]

On Thu, May 30, 2024 at 07:53:42PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Thu, May 30, 2024 at 02:14:20AM +0200, Andreas Rheinhardt wrote:
> >> Forgotten in 65ddc74988245a01421a63c5cffa4d900c47117c.
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> >> ---
> >>  libavformat/nutdec.c | 14 ++++----------
> >>  1 file changed, 4 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> >> index 0bb7f154db..34b7e3cb9a 100644
> >> --- a/libavformat/nutdec.c
> >> +++ b/libavformat/nutdec.c
> >> @@ -881,8 +881,6 @@ static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int
> >>      int count = ffio_read_varlen(bc);
> >>      int skip_start = 0;
> >>      int skip_end = 0;
> >> -    int channels = 0;
> >> -    int64_t channel_layout = 0;
> >>      int sample_rate = 0;
> >>      int width = 0;
> >>      int height = 0;
> >> @@ -930,7 +928,7 @@ static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int
> >>                  AV_WB64(dst, v64);
> >>                  dst += 8;
> >>              } else if (!strcmp(name, "ChannelLayout") && value_len == 8) {
> >> -                channel_layout = avio_rl64(bc);
> >> +                // Ignored
> >>                  continue;
> >>              } else {
> >>                  av_log(s, AV_LOG_WARNING, "Unknown data %s / %s\n", name, type_str);
> >> @@ -952,7 +950,7 @@ static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int
> >>              } else if (!strcmp(name, "SkipEnd")) {
> >>                  skip_end = value;
> >>              } else if (!strcmp(name, "Channels")) {
> >> -                channels = value;
> >> +                // Ignored
> >>              } else if (!strcmp(name, "SampleRate")) {
> >>                  sample_rate = value;
> >>              } else if (!strcmp(name, "Width")) {
> >> @@ -965,18 +963,14 @@ static int read_sm_data(AVFormatContext *s, AVIOContext *bc, AVPacket *pkt, int
> >>          }
> >>      }
> >>  
> >> -    if (channels || channel_layout || sample_rate || width || height) {
> >> -        uint8_t *dst = av_packet_new_side_data(pkt, AV_PKT_DATA_PARAM_CHANGE, 28);
> >> +    if (sample_rate || width || height) {
> >> +        uint8_t *dst = av_packet_new_side_data(pkt, AV_PKT_DATA_PARAM_CHANGE, 16);
> >>          if (!dst)
> >>              return AVERROR(ENOMEM);
> >>          bytestream_put_le32(&dst,
> >>                              AV_SIDE_DATA_PARAM_CHANGE_SAMPLE_RATE*(!!sample_rate) +
> >>                              AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS*(!!(width|height))
> >>                             );
> >> -        if (channels)
> >> -            bytestream_put_le32(&dst, channels);
> >> -        if (channel_layout)
> >> -            bytestream_put_le64(&dst, channel_layout);
> >>          if (sample_rate)
> >>              bytestream_put_le32(&dst, sample_rate);
> >>          if (width || height){
> > 
> > This would break mid stream changes to the channel layout & channels when it
> > is carried at format level only
> > 
> > The commit message also does not adequately explain why such mid stream changes
> > are ignored
> > 
> 
> Mid-stream changes like this have been deprecated in
> 09b5d3fb44ae1036700f80c8c80b15e9074c58c3;
> 65ddc74988245a01421a63c5cffa4d900c47117c removed it, but only
> incompletely: The side data flags for channel count and channel layout
> changes were no longer written (in fact, they were removed from
> packet.h), yet it still wrote the rest of the side data as if these
> flags existed and had been written. That is the inconsistency this
> commit addresses. It does not address whether channel count/layout
> updates should have been removed, because that has already happened.

i honestly belive that we should support changing channel(layout) for
cases like PCM in nut

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240530/caa93247/attachment.sig>