| Message ID | 20180308160208.2688-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On 3/8/2018 4:02 PM, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/extract_extradata_bsf.c | 4 ++++ > 1 file changed, 4 insertions(+) Can't most (or all) of these be fixed by using av_mallocz? - Derek
On 3/9/2018 12:18 PM, Derek Buitenhuis wrote: > On 3/8/2018 4:02 PM, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/extract_extradata_bsf.c | 4 ++++ >> 1 file changed, 4 insertions(+) > > Can't most (or all) of these be fixed by using av_mallocz? Yes, but it's slower, and the buffer is guaranteed to be written to with actual data after being allocated. This is a filter that may run once per processed packet, so the less overhead the better.
On 3/9/2018 3:22 PM, James Almer wrote: > Yes, but it's slower, and the buffer is guaranteed to be written to with > actual data after being allocated. > > This is a filter that may run once per processed packet, so the less > overhead the better. Not sure I buy the "speed" argument here, but OK. - Derek
On 3/9/2018 12:47 PM, Derek Buitenhuis wrote: > On 3/9/2018 3:22 PM, James Almer wrote: >> Yes, but it's slower, and the buffer is guaranteed to be written to with >> actual data after being allocated. >> >> This is a filter that may run once per processed packet, so the less >> overhead the better. > > Not sure I buy the "speed" argument here, but OK. > > - Derek There's really nothing to win zeroing then immediately writing actual data on top of it. Pushed, thanks.
On Fri, 9 Mar 2018 13:31:45 -0300 James Almer <jamrial@gmail.com> wrote: > On 3/9/2018 12:47 PM, Derek Buitenhuis wrote: > > On 3/9/2018 3:22 PM, James Almer wrote: > >> Yes, but it's slower, and the buffer is guaranteed to be written to with > >> actual data after being allocated. > >> > >> This is a filter that may run once per processed packet, so the less > >> overhead the better. > > > > Not sure I buy the "speed" argument here, but OK. > > > > - Derek > > There's really nothing to win zeroing then immediately writing actual > data on top of it. > Except code clarity and robustness?
On 3/9/18, wm4 <nfxjfg@googlemail.com> wrote: > On Fri, 9 Mar 2018 13:31:45 -0300 > James Almer <jamrial@gmail.com> wrote: > >> On 3/9/2018 12:47 PM, Derek Buitenhuis wrote: >> > On 3/9/2018 3:22 PM, James Almer wrote: >> >> Yes, but it's slower, and the buffer is guaranteed to be written to >> >> with >> >> actual data after being allocated. >> >> >> >> This is a filter that may run once per processed packet, so the less >> >> overhead the better. >> > >> > Not sure I buy the "speed" argument here, but OK. >> > >> > - Derek >> >> There's really nothing to win zeroing then immediately writing actual >> data on top of it. >> > > Except code clarity and robustness? Code already is clear and robust enough.
On Fri, Mar 9, 2018 at 9:52 PM, wm4 <nfxjfg@googlemail.com> wrote: > On Fri, 9 Mar 2018 13:31:45 -0300 > James Almer <jamrial@gmail.com> wrote: > >> On 3/9/2018 12:47 PM, Derek Buitenhuis wrote: >> > On 3/9/2018 3:22 PM, James Almer wrote: >> >> Yes, but it's slower, and the buffer is guaranteed to be written to with >> >> actual data after being allocated. >> >> >> >> This is a filter that may run once per processed packet, so the less >> >> overhead the better. >> > >> > Not sure I buy the "speed" argument here, but OK. >> > >> > - Derek >> >> There's really nothing to win zeroing then immediately writing actual >> data on top of it. >> > > Except code clarity and robustness? We use this sort of pattern in all sorts of places where buffers are read and padded. Its clear enough. - Hendrik
On Fri, 9 Mar 2018 21:57:38 +0100 Hendrik Leppkes <h.leppkes@gmail.com> wrote: > On Fri, Mar 9, 2018 at 9:52 PM, wm4 <nfxjfg@googlemail.com> wrote: > > On Fri, 9 Mar 2018 13:31:45 -0300 > > James Almer <jamrial@gmail.com> wrote: > > > >> On 3/9/2018 12:47 PM, Derek Buitenhuis wrote: > >> > On 3/9/2018 3:22 PM, James Almer wrote: > >> >> Yes, but it's slower, and the buffer is guaranteed to be written to with > >> >> actual data after being allocated. > >> >> > >> >> This is a filter that may run once per processed packet, so the less > >> >> overhead the better. > >> > > >> > Not sure I buy the "speed" argument here, but OK. > >> > > >> > - Derek > >> > >> There's really nothing to win zeroing then immediately writing actual > >> data on top of it. > >> > > > > Except code clarity and robustness? > > We use this sort of pattern in all sorts of places where buffers are > read and padded. Its clear enough. Well, the fact that there are patches every once in a while that fix places where this was not correctly done could hint otherwise. Not that I want to make a huge discussion about it (though I always disliked this padding requirement).
diff --git a/libavcodec/extract_extradata_bsf.c b/libavcodec/extract_extradata_bsf.c index d40907a675..0bffe8f42c 100644 --- a/libavcodec/extract_extradata_bsf.c +++ b/libavcodec/extract_extradata_bsf.c @@ -114,6 +114,7 @@ static int extract_extradata_h2645(AVBSFContext *ctx, AVPacket *pkt, ret = AVERROR(ENOMEM); goto fail; } + memset(extradata + extradata_size, 0, AV_INPUT_BUFFER_PADDING_SIZE); *data = extradata; *size = extradata_size; @@ -169,6 +170,7 @@ static int extract_extradata_vc1(AVBSFContext *ctx, AVPacket *pkt, return AVERROR(ENOMEM); memcpy(*data, pkt->data, extradata_size); + memset(*data + extradata_size, 0, AV_INPUT_BUFFER_PADDING_SIZE); *size = extradata_size; if (s->remove) { @@ -199,6 +201,7 @@ static int extract_extradata_mpeg12(AVBSFContext *ctx, AVPacket *pkt, return AVERROR(ENOMEM); memcpy(*data, pkt->data, *size); + memset(*data + *size, 0, AV_INPUT_BUFFER_PADDING_SIZE); if (s->remove) { pkt->data += *size; @@ -228,6 +231,7 @@ static int extract_extradata_mpeg4(AVBSFContext *ctx, AVPacket *pkt, return AVERROR(ENOMEM); memcpy(*data, pkt->data, *size); + memset(*data + *size, 0, AV_INPUT_BUFFER_PADDING_SIZE); if (s->remove) { pkt->data += *size;
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/extract_extradata_bsf.c | 4 ++++ 1 file changed, 4 insertions(+)