On 9/25/2017 2:29 PM, wm4 wrote: > On Mon, 25 Sep 2017 14:07:54 -0300 > James Almer <jamr...@gmail.com> wrote: > >> On 9/25/2017 1:43 PM, wm4 wrote: >>> On Mon, 25 Sep 2017 10:58:31 -0300 >>> James Almer <jamr...@gmail.com> wrote: >>> >>>>> Using av_packet_copy_props() instead of introducing yet another weird >>>>> function would be better. >>>> >>>> It can't be used in some cases. Look at the last two patches in the set. >>>> copy_props can't be used there without some extra pointless work. >>> >>> Well, you need to copy the props first, before you write any fields >>> that are touched by the function. >> >> How so? The function only looks at side_data and side_data_size from a >> const src packet, then writes those same two fields to the dst packet >> if copying was successful. av_packet_free_side_data() also only cares >> about those two fields. >> >> I don't know why whoever wrote the code in ffmpeg.c and movenc.c didn't >> use copy_props(), but a quick read hints they didn't want to copy any >> other property except side data, apparently as it would break whatever >> pts/duration calculations they were doing with their tmp packets. > > So, copy pts/duration after "copying" (ref-ing) the entire packet. I > have to do similar things in my code with AVFrame.
ffmpeg.c in this case doesn't ref the packet. It inits it, writes some fields based on (but without directly copying them from) the source packet, then copies the side data. If you really don't want functions this specific and are willing to fix the two cases in ffmpeg.c and movenc.c to use copy_props then i can repurpose this patchset to only deprecate the existing av_copy_packet_side_data(). Keep in mind however that by deprecating and removing said function without adding a direct replacement we'll probably be annoying existing users of said function (Chromium is one it seems) by forcing them to also work around the extra stuff copy_props() would do to their packets. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
