On Mon, 25 Sep 2017 14:37:52 -0300 James Almer <jamr...@gmail.com> wrote:
> 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. What exactly is the trouble of just moving setting pts/duration below the function call? Also adding a function with very similar name and subtly different semantics (whose differences aren't even explained) is a very bad idea. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
