[FFmpeg-devel] [PATCH 1/5] fftools/ffmpeg: Fix forward CPB props in to out

[Michael Niedermayer]

On Mon, Dec 16, 2019 at 10:19:29PM +0000, Gaullier Nicolas wrote:
> 
> >> @@ -3562,12 +3562,14 @@ static int init_output_stream(OutputStream *ost, char *error, int error_len)
> >>              int i;
> >>              for (i = 0; i < ist->st->nb_side_data; i++) {
> >>                  AVPacketSideData *sd = &ist->st->side_data[i];
> >> +                if (sd->type != AV_PKT_DATA_CPB_PROPERTIES) {
> >>                  uint8_t *dst = av_stream_new_side_data(ost->st, sd->type, sd->size);
> >>                  if (!dst)
> >>                      return AVERROR(ENOMEM);
> >>                  memcpy(dst, sd->data, sd->size);
> >>                  if (ist->autorotate && sd->type == AV_PKT_DATA_DISPLAYMATRIX)
> >>                      av_display_rotation_set((uint32_t *)dst, 0);
> >> +                }
> >>              }
> >>          }
> >>
> >
> >Can you clarify why you are exlcuding CBP side-data from being copied here? It seems to not match the commit message in my mind.
> >Note that from some containers, namely mov/mp4, the container can actually provide CBP data, which means the stream CBP data would be valid and potentially relevant.
> 
> This portion of the code is for when the stream is transcoded, so there is no relation between the input and output CPB.
> But in init_output_stream_streamcopy() you can see that all side data is actually copied (it was already there, before this patchset, and I left it untouched) with no exception (=incl. CPB). And this is what I was looking for: being able to forward the input CPB size to the output, when stream-copying.
> 
> Note I only modified the CPB behaviour to restrict the scope of this patchset, but there are certainly other side data that should not be forwarded from input to output; this does not sound easy for me and maybe it should be discussed, but this patchset is consistent as is and I think it would require other patches if we want something more generic about what side data should be forwarded or not.
> 
> This is my understanding and I may have missed something of course. I have mainly tested mpeg2 encoding and stream-copying and checked fate.

please add this information to the commit message

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20191217/f4b47236/attachment.sig>