<div dir="ltr"><div><div>Ahhh ok, I got it wrong, I didn't use pOutputPortPrivate / pInputPortPrivate with my player and I read the headers too quicky...<br></div>Now that you explained, it's all clear.<br><br></div>
<div>I did that patch because of PATCH 13/13.<br>I
needed to add an other FIFO so I searched for an other unused pointer
in OMX_BUFFERHEADERTYPE and I found pOutputPortPrivate. Then I see (but wrongly) that pOutputPortPrivate should have been used too here.<br></div><div><br></div>So I guess I have to rework the Patch 13/13 in order to use an other pointer for the FIFO.<br>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-06-29 22:33 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 Thu, 26 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">
For output buffers, pInputPortPrivate is already used by the OMX_FIFO.<br>
<br>
It was safe for the current state. Indeed, the output fifo had never more than<br>
one elements, so pInputPortPrivate was always NULL, and then pBuffer was never<br>
overwritten in OmxFillBufferDone.<br>
---<br>
modules/codec/omxil/omxil.c | 6 +++---<br>
1 file changed, 3 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/modules/codec/omxil/omxil.c b/modules/codec/omxil/omxil.c<br>
index ea3d7e2..75196b3 100644<br>
--- a/modules/codec/omxil/omxil.c<br>
+++ b/modules/codec/omxil/omxil.c<br>
@@ -1281,7 +1281,7 @@ static picture_t *DecodeVideo( decoder_t *p_dec, block_t **pp_block )<br>
<br>
OMX_FIFO_GET(&p_sys->out.fifo, p_header);<br>
p_header->pAppPrivate = p_next_pic;<br>
- p_header->pInputPortPrivate = p_header->pBuffer;<br>
+ p_header->pOutputPortPrivate = p_header->pBuffer;<br>
p_header->pBuffer = p_next_pic->p[0].p_pixels;<br>
}<br>
else<br>
@@ -1744,9 +1744,9 @@ static OMX_ERRORTYPE OmxFillBufferDone( OMX_HANDLETYPE omx_handle,<br>
(int)omx_header->nFilledLen, FromOmxTicks(omx_header-><u></u>nTimeStamp) );<br>
#endif<br>
<br>
- if(omx_header-><u></u>pInputPortPrivate)<br>
+ if(omx_header-><u></u>pOutputPortPrivate)<br>
{<br>
- omx_header->pBuffer = omx_header->pInputPortPrivate;<br>
+ omx_header->pBuffer = omx_header-><u></u>pOutputPortPrivate;<br>
}<br>
OMX_FIFO_PUT(&p_sys->out.fifo, omx_header);<br>
<br>
-- <br>
1.7.10.4<br>
</blockquote>
<br></div></div>
This is wrong.<br>
<br>
The pOutputPortPrivate field is private to the output port of the codec - we can't use it for anything, since the codec might be using this field for storing other data associated with the buffer.<br>
<br>
I don't see any problem with the current code though. The fifo only uses pInputPortPrivate while the buffer actually is stored within the fifo. When we take the buffer out from the fifo, we can reuse pInputPortPrivate for anything we want - only once we put the buffer back into the fifo, the field will be overwritten, but this doesn't matter since we don't read it when we take it out from the fifo again.<br>
<br>
So I don't really see any issue here, and your patch would certainly break things.<br>
<br>
Do you agree, or can you explain the flow where memory would be corrupted?<div class=""><br>
<br>
In your commit message, you said:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
It was safe for the current state. Indeed, the output fifo had never more than<br>
one elements, so pInputPortPrivate was always NULL, and then pBuffer was never<br>
overwritten in OmxFillBufferDone.<br>
</blockquote>
<br></div>
You have the control flow backwards here - the pInputPortPrivate field only gets touched while it is in the fifo, i.e. between the OMX_FIFO_PUT call, until the OMX_FIFO_GET call. After OMX_FIFO_GET, we should consider pInputPortPrivate overwritten to any random value - but at that point we don't read it, we only overwrite it to store the pBuffer pointer. Then we pass the buffer to the codec, which will not touch the pInputPortPrivate field, and when the codec returns the buffer to us, we'll move the value back from pInputPortPrivate to pBuffer, and give it to OMX_FIFO_PUT which then again overwrites pInputPortPrivate.<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>