| Message ID | 20190621141552.21430-1-derek.buitenhuis@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On 6/21/2019 11:15 AM, Derek Buitenhuis wrote: > This packet was not necessarily unreferenced. > > Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com> > --- > fftools/ffprobe.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c > index 3becb6330e..dac70ba5a1 100644 > --- a/fftools/ffprobe.c > +++ b/fftools/ffprobe.c > @@ -2429,6 +2429,8 @@ static int read_interval_packets(WriterContext *w, InputFile *ifile, > } > av_packet_unref(&pkt); > } > + av_packet_unref(&pkt); > + Remove the three lines below as well before pushing. They are superfluous as av_packet_unref() does the same internally. > av_init_packet(&pkt); > pkt.data = NULL; > pkt.size = 0; >
Derek Buitenhuis (12019-06-21): > This packet was not necessarily unreferenced. > > Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com> > --- > fftools/ffprobe.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c > index 3becb6330e..dac70ba5a1 100644 > --- a/fftools/ffprobe.c > +++ b/fftools/ffprobe.c > @@ -2429,6 +2429,8 @@ static int read_interval_packets(WriterContext *w, InputFile *ifile, > } > av_packet_unref(&pkt); > } > + av_packet_unref(&pkt); > + How can the packet not be unreferenced when the very previous instruction is av_packet_unref()? All the code paths I see either pass through the existing av_packet_unref() before reaching the new one or arrive with a blank packet. Am I missing something? > av_init_packet(&pkt); > pkt.data = NULL; > pkt.size = 0; Regards,
On 21/06/2019 15:26, Nicolas George wrote: > How can the packet not be unreferenced when the very previous > instruction is av_packet_unref()? All the code paths I see either pass > through the existing av_packet_unref() before reaching the new one or > arrive with a blank packet. Am I missing something? The previous instructions is not an unref if it hits one of the breaks in the loop above. - Derek
Derek Buitenhuis (12019-06-21): > The previous instructions is not an unref if it hits one of the breaks > in the loop above. Of course. Sorry for wasting your time. Regards,
On 21/06/2019 15:26, James Almer wrote: > Remove the three lines below as well before pushing. They are > superfluous as av_packet_unref() does the same internally. OK. The documentation for av_packet_unref says it sets the 'remaining' fields to default values, but av_init_packet says it sets the 'optional' fields to default values. Not clear if they mean the same thing. Looking at the source, it's clear, but I prefer not to do that, since it encourages relying on undocumented behavior. - Derek
On 6/21/2019 11:39 AM, Derek Buitenhuis wrote: > On 21/06/2019 15:26, James Almer wrote: >> Remove the three lines below as well before pushing. They are >> superfluous as av_packet_unref() does the same internally. > > OK. > > The documentation for av_packet_unref says it sets the 'remaining' > fields to default values, but av_init_packet says it sets the > 'optional' fields to default values. Not clear if they mean > the same thing. Looking at the source, it's clear, but I prefer > not to do that, since it encourages relying on undocumented > behavior. > > - Derek The doxy says "Unreference the buffer referenced by the packet and reset the remaining packet fields to their default values", which means it unrefs the AVBufferRef, and then sets the remaining fields in the packet to default values, effectively wiping the whole struct. av_init_packet() does indeed change only a subset of fields, as stated by the doxy, which is why the data and size fields are set to NULL/0 in this function right after it, as those are not touched by it. Since you want to wipe the packet, calling av_packet_unref() on it does a thorough job.
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c index 3becb6330e..dac70ba5a1 100644 --- a/fftools/ffprobe.c +++ b/fftools/ffprobe.c @@ -2429,6 +2429,8 @@ static int read_interval_packets(WriterContext *w, InputFile *ifile, } av_packet_unref(&pkt); } + av_packet_unref(&pkt); + av_init_packet(&pkt); pkt.data = NULL; pkt.size = 0;
This packet was not necessarily unreferenced. Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com> --- fftools/ffprobe.c | 2 ++ 1 file changed, 2 insertions(+)