<div dir="ltr">On Fri, Jul 4, 2014 at 10:00 AM, Rémi Denis-Courmont <span dir="ltr"><<a href="mailto:remi@remlab.net" target="_blank">remi@remlab.net</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Le 2014-07-04 16:54, Tristan Matthews a écrit :<div><div class="h5"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Fixes #11605<br>
---<br>
modules/codec/avcodec/encoder.<u></u>c | 31 +++++++++++-------------------<u></u>-<br>
1 file changed, 11 insertions(+), 20 deletions(-)<br>
<br>
diff --git a/modules/codec/avcodec/<u></u>encoder.c<br>
b/modules/codec/avcodec/<u></u>encoder.c<br>
index a6fcf66..3646b6e 100644<br>
--- a/modules/codec/avcodec/<u></u>encoder.c<br>
+++ b/modules/codec/avcodec/<u></u>encoder.c<br>
@@ -1013,19 +1013,6 @@ static block_t *EncodeVideo( encoder_t *p_enc,<br>
picture_t *p_pict )<br>
{<br>
encoder_sys_t *p_sys = p_enc->p_sys;<br>
int i_plane;<br>
- /* Initialize the video output buffer the first time.<br>
- * This is done here instead of OpenEncoder() because we need the actual<br>
- * bits_per_pixel value, without having to assume anything.<br>
- */<br>
- const int bitsPerPixel = p_enc->fmt_out.video.i_bits_<u></u>per_pixel ?<br>
- p_enc->fmt_out.video.i_bits_<u></u>per_pixel :<br>
- p_sys->p_context->bits_per_<u></u>coded_sample ?<br>
- p_sys->p_context->bits_per_<u></u>coded_sample :<br>
- 24;<br>
- const int blocksize = __MAX( FF_MIN_BUFFER_SIZE, ( bitsPerPixel<br>
* p_sys->p_context->height * p_sys->p_context->width ) / 8 + 200 );<br>
- block_t *p_block = block_Alloc( blocksize );<br>
- if( unlikely(p_block == NULL) )<br>
- return NULL;<br>
<br>
AVFrame *frame = NULL;<br>
if( likely(p_pict) ) {<br>
@@ -1090,7 +1077,6 @@ static block_t *EncodeVideo( encoder_t *p_enc,<br>
picture_t *p_pict )<br>
{<br>
msg_Warn( p_enc, "almost fed libavcodec with two<br>
frames with "<br>
"the same PTS (%"PRId64 ")", frame->pts );<br>
- block_Release( p_block );<br>
return NULL;<br>
}<br>
else if ( p_sys->i_last_pts > frame->pts )<br>
@@ -1098,7 +1084,6 @@ static block_t *EncodeVideo( encoder_t *p_enc,<br>
picture_t *p_pict )<br>
msg_Warn( p_enc, "almost fed libavcodec with a frame<br>
in the "<br>
"past (current: %"PRId64 ", last: %"PRId64")",<br>
frame->pts, p_sys->i_last_pts );<br>
- block_Release( p_block );<br>
return NULL;<br>
}<br>
else<br>
@@ -1108,21 +1093,27 @@ static block_t *EncodeVideo( encoder_t<br>
*p_enc, picture_t *p_pict )<br>
frame->quality = p_sys->i_quality;<br>
}<br>
<br>
- AVPacket av_pkt;<br>
+ AVPacket av_pkt = {0};<br>
</blockquote>
<br></div></div>
Why is this needed now?</blockquote><div><br></div><div>av_pkt.data and av_pkt.size must now be initialized to 0 before being passed to avcodec_encode_video2. It could be done explicitly (i.e. av_pkt.size = 0 av_pkt.data = NULL), I just wrote it this way to be consistent with the style used in EncodeAudio.<br>
<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><br></div><div class="">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
int is_data;<br>
<br>
av_init_packet( &av_pkt );<br>
- av_pkt.data = p_block->p_buffer;<br>
- av_pkt.size = p_block->i_buffer;<br>
<br>
if( avcodec_encode_video2( p_sys->p_context, &av_pkt, frame,<br>
&is_data ) < 0<br>
|| is_data == 0 )<br>
{<br>
- block_Release( p_block );<br>
return NULL;<br>
}<br>
<br>
- p_block->i_buffer = av_pkt.size;<br>
+ block_t *p_block = block_Alloc( av_pkt.size );<br>
+ if( unlikely(p_block == NULL) )<br>
+ {<br>
+ av_free_packet( &av_pkt );<br>
+ return NULL;<br>
+ }<br>
+<br>
+ memcpy( p_block->p_buffer, av_pkt.data, av_pkt.size );<br>
+ av_free_packet( &av_pkt );<br>
</blockquote>
<br></div>
"memcpy() is evil." IMHO, the AVPacket should be wrapped into a block_t rather than copied.</blockquote><div><br></div><div>Yeah I had the same thought, but I wasn't sure if it was possible. Aside from overloading the block's pf_release function pointer to call av_free_packet, how could the AVPacket be wrapped without breaking callers? Do we have any examples of this kind of thing already in VLC?<br>
<br>Best,<br>Tristan<br></div></div></div></div>