Opened 18 months ago

Last modified 18 months ago

#5551 new defect

av_bsf_send_packet can't take ownership on not reference-counted packets

Reported by: mrlika Owned by:
Priority: normal Component: avcodec
Version: git-master Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Because it doesn't know how to release it. And caller also doesn't know when to delete the packet passed to black-box algorithm.

For not reference-counted packets it is more correct to create referenced-counted copy of the packet and leave ownership to the caller.

For example av_grow_packet h264_mp4toannexb and h265_mp4toannexb already call av_grow_packet on input packets that just overwrites pkt->data field with newly allocated buffer for not reference-counted packets. It leads to memory leak.

Change History (6)

comment:1 Changed 18 months ago by mrlika

  • Component changed from undetermined to avcodec
  • Version changed from unspecified to git-master

comment:2 follow-up: Changed 18 months ago by gjdfgh

Simple: don't pass non-refcounted packets to these APIs.

comment:3 in reply to: ↑ 2 Changed 18 months ago by mrlika

Replying to gjdfgh:

Simple: don't pass non-refcounted packets to these APIs.

Yes it is simple but not obvious unless you investigate the source code. And in this case it is better to validate input packets for reference-countness.

comment:4 Changed 18 months ago by gjdfgh

Maybe it should be better documented. Still, "taking ownership" implies the user can't access the packet after passing it to the API.

comment:5 Changed 18 months ago by mrlika

I see the following ways to fix this:

1) Restrict to use the API with not reference-counted packets by updating documentation and\or adding asserts validation for reference-countness.

2) Create referenced-counted copy of not reference-counted packets and leave ownership to the caller. Update documentation.

3) Change API to always leave paket ownership to the caller. Update documentation.

comment:6 Changed 18 months ago by mrlika

And latst variant:

0) Do not touch anything

Do you have any thoughts on this?

Note: See TracTickets for help on using tickets.