<div dir="ltr">AH OK, I thought the codec should have returned all buffers at that state.<div><br></div><div><div>I modified that part of the code because I had a raise condition between omx </div><div>free and HWbuffer free, but then I reworked lot of stuffs. So I should be able</div>
<div>to put back the original code and have HWbuffer free working.</div></div><div><br></div><div>Thomas.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-06-24 18:31 GMT+02:00 Martin Storsjö <span dir="ltr"><<a href="mailto:martin@martin.st" target="_blank">martin@martin.st</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, 24 Jun 2014, Thomas Guillem wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
---<br>
modules/codec/omxil/omxil.c |  236 ++++++++++++++++++++++--------<u></u>-------------<br>
1 file changed, 123 insertions(+), 113 deletions(-)<br>
<br>
diff --git a/modules/codec/omxil/omxil.c b/modules/codec/omxil/omxil.c<br>
index df75c93..c1ed20d 100644<br>
--- a/modules/codec/omxil/omxil.c<br>
+++ b/modules/codec/omxil/omxil.c<br>
@@ -355,6 +355,113 @@ static OMX_ERRORTYPE UpdatePixelAspect(decoder_t *p_dec)<br>
    return omx_err;<br>
}<br>
<br>
+static OMX_ERRORTYPE AllocateBuffers(decoder_t *p_dec, OmxPort *p_port)<br>
+{<br>
+    decoder_sys_t *p_sys = p_dec->p_sys;<br>
+    OMX_ERRORTYPE omx_error = OMX_ErrorUndefined;<br>
+    OMX_PARAM_PORTDEFINITIONTYPE *def = &p_port->definition;<br>
+    unsigned int i;<br>
+<br>
+#ifdef OMXIL_EXTRA_DEBUG<br>
+    msg_Dbg( p_dec, "AllocateBuffers(%d)", def->eDir );<br>
+#endif<br>
+<br>
+    p_port->i_buffers = p_port->definition.<u></u>nBufferCountActual;<br>
+<br>
+    p_port->pp_buffers = calloc(p_port->i_buffers, sizeof(OMX_BUFFERHEADERTYPE*))<u></u>;<br>
+    if( !p_port->pp_buffers )<br>
+    {<br>
+        return OMX_<u></u>ErrorInsufficientResources;<br>
+    }<br>
+<br>
+    for(i = 0; i < p_port->i_buffers; i++)<br>
+    {<br>
+#if 0<br>
+#define ALIGN(x,BLOCKLIGN) (((x) + BLOCKLIGN - 1) & ~(BLOCKLIGN - 1))<br>
+        char *p_buf = malloc(p_port->definition.<u></u>nBufferSize +<br>
+                             p_port->definition.<u></u>nBufferAlignment);<br>
+        p_port->pp_buffers[i] = (void *)ALIGN((uintptr_t)p_buf, p_port->definition.<u></u>nBufferAlignment);<br>
+#endif<br>
+<br>
+        if(p_port->b_direct)<br>
+            omx_error =<br>
+                OMX_UseBuffer( p_sys->omx_handle, &p_port->pp_buffers[i],<br>
+                               p_port->i_port_index, 0,<br>
+                               p_port->definition.<u></u>nBufferSize, (void*)1);<br>
+        else<br>
+            omx_error =<br>
+                OMX_AllocateBuffer( p_sys->omx_handle, &p_port->pp_buffers[i],<br>
+                                    p_port->i_port_index, 0,<br>
+                                    p_port->definition.<u></u>nBufferSize);<br>
+<br>
+        if(omx_error != OMX_ErrorNone) break;<br>
+        OMX_FIFO_PUT(&p_port->fifo, p_port->pp_buffers[i]);<br>
+    }<br>
+    CHECK_ERROR(omx_error, "AllocateBuffers failed (%x, %i)",<br>
+                omx_error, (int)p_port->i_port_index );<br>
+<br>
+<br>
+#ifdef OMXIL_EXTRA_DEBUG<br>
+    msg_Dbg( p_dec, "AllocateBuffers(%d)::done", def->eDir );<br>
+#endif<br>
+error:<br>
+    return omx_error;<br>
+}<br>
+<br>
+static OMX_ERRORTYPE FreeBuffers(decoder_t *p_dec, OmxPort *p_port)<br>
+{<br>
+    decoder_sys_t *p_sys = p_dec->p_sys;<br>
+    OMX_PARAM_PORTDEFINITIONTYPE *def = &p_port->definition;<br>
+    OMX_ERRORTYPE omx_error = OMX_ErrorNone;<br>
+    OMX_BUFFERHEADERTYPE *p_buffer;<br>
+    unsigned int i;<br>
+<br>
+#ifdef OMXIL_EXTRA_DEBUG<br>
+    msg_Dbg( p_dec, "FreeBuffers(%d)", def->eDir );<br>
+#endif<br>
+<br>
+    /* wait for omx to release all buffers */<br>
+    while (1) {<br>
+        OMX_FIFO_PEEK(&p_port->fifo, p_buffer);<br>
+        if (!p_buffer) break;<br>
+<br>
+        OMX_FIFO_GET(&p_port->fifo, p_buffer);<br>
+        if (p_buffer->nFlags & SENTINEL_FLAG) {<br>
+            free(p_buffer);<br>
+            continue;<br>
+        }<br>
+        msg_Dbg( p_dec, "Stray buffer left in fifo, %p", p_buffer );<br>
+    }<br>
+<br>
+    for(i = 0; i < p_port->i_buffers; i++)<br>
+    {<br>
+        p_buffer =  p_port->pp_buffers[i];<br>
+<br>
+        if( p_buffer )<br>
+        {<br>
+            if( p_buffer->pAppPrivate != NULL )<br>
+                decoder_DeletePicture( p_dec, p_buffer->pAppPrivate );<br>
+            omx_error = OMX_FreeBuffer( p_sys->omx_handle,<br>
+                                        p_port->i_port_index, p_buffer );<br>
+            if(omx_error != OMX_ErrorNone) break;<br>
+        }<br>
+    }<br>
</blockquote>
<br></div></div>
Why did you change the order of this code compared to how it was before? This will not work as intended. Previously this was structured like this:<br>
<br>
    for (0..i_buffers) {<br>
        OMX_FIFO_GET<br>
    }<br>
    while (1)<br>
        // Get any remaining buffer (sentinels?)<br>
<br>
Now you just remove any buffers currently in the FIFO, and then assume you have got them all.<br>
<br>
At this point, we've signalled the codec to return them to us (by setting the state to loaded, or by disabling the port), but the codec might not have returned all of them yet - and while the codec still owns one of the buffers, we cannot free it. (I've spent a lot of time debugging quite non-obvious bugs relating exactly to this piece of code.)<br>

<br>
Thus, we need to make sure that we actually remove exactly 'i_buffers' buffers from the fifo. (And if there are sentinel buffers in the fifo those should not be counted.) By looping 'i_buffers' times and doing OMX_FIFO_GET each time, we make sure the code blocks until there is one buffer that we can free.<br>

<br>
Thus, please keep the original structure of the code that frees the buffers - but factorizing it definitely is a good step.<br>
<br>
// Martin<br>
______________________________<u></u>_________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="https://mailman.videolan.org/listinfo/vlc-devel" target="_blank">https://mailman.videolan.org/<u></u>listinfo/vlc-devel</a><br>
</blockquote></div><br></div>

<br>
<p><span style="color:rgb(136,136,136);font-family:arial,sans-serif;line-height:18.1875px;background-color:rgb(255,255,255)">This email and any files transmitted with it are confidential and are intended solely for the use of the individual or entity to which they are addressed. Access to this e-mail by anyone else is unauthorised. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. E-mail messages are not necessarily secure. Archos does not accept responsibility for any changes made to this message after it was sent.</span></p>