Quoting Andreas Rheinhardt (2020-05-11 23:58:47) > Anton Khirnov: > > Quoting Andreas Rheinhardt (2020-05-10 21:35:54) > >> Anton Khirnov: > >>> Quoting Marton Balint (2020-05-10 19:45:04) > >>>> > >>>> > >>>> On Sun, 10 May 2020, Anton Khirnov wrote: > >>>> > >>>>> Quoting Andreas Rheinhardt (2020-05-08 00:55:00) > >>>>>> This commit fixes two recent regressions both of which are about using > >>>>>> pkt->stream_index as index in an AVFormatContext's streams array before > >>>>>> actually comparing the value with the count of streams in said array. > >>>>>> 96e5e6abb9851d7a26ba21703955d5826ac857c0 did this in > >>>>>> prepare_input_packet() and 64063512227c4c87a7d16a1076481dc6baf19841 did > >>>>>> likewise in write_packets_common(). > >>>>>> > >>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> > >>>>>> --- > >>>>>> The same error in the same file applied on the same day by two > >>>>>> different > >>>>>> people. How unlikely. > >>>>> > >>>>> How is it a regression? Isn't it rather invalid API use? > >>>> > >>>> Fun fact: 7b03b65bf0d02519c86750d2da33f413e11cf0c6 > >>>> > >>>> Yes, it is kind of invalid API use, but since the check is already > >>>> there, > >>>> we should make it actually worthwile. > >>> > >>> lol > >>> > >>> I agree that checking for it is a good idea, obviously, but I wouldn't > >>> call it a regression. > >>> > >> How about rephrasing the first sentence to: "This commit stops using > >> pkt->stream_index as index in an AVFormatContext's streams array before > >> actually comparing the value with the count of streams in said array." > > > > Sure, sounds good. > > > >>>> > >>>>> > >>>>> Not that I object to having a check. But then why is check_packet() > >>>>> called so deep and not immediately on entry to the muxer? > >>>> > >>>> I guess it is not that deep, but recent factorization efforts hidden it > >>>> a > >>>> bit. > >>> > >>> You can see in my original commit it is the very first thing done after > >>> entering the muxer. Right now it's several function calls deep. > >>> > >> I could make it the very first thing called in write_packets_common(). > > > > Why not move the check_packet() call out of prepare_packet() into > > av_[interleaved_]write_frame() instead? > > > This would add code duplication and IMO it is nicer to only check a > packet for validity if there is actually a packet to check.
Okay. I don't entirely agree, but this is not important enough to waste time arguing about. Feel free to push. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
