[Bug 751334] FLAC: memory leak on a specific media file

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=751334

--- Comment #27 from Vineeth <vineeth.tm at samsung.com> ---
(In reply to Nicolas Dufresne (stormer) from comment #24)
> I have the feeling this bug is going in the wrong direction. First, libflac
> need to parse the header which are both in the stream and in the caps. If
> you where to do it from caps and again from the stream, I can understand
> libflac getting confused. There is benefits of doing from caps, one if that
> if it fails, you can fail on caps event, and that will let decodebin pick
> the next flac decoder in queue (if you are lucky to have one).
> 

This will work only with decodebin right?
If i still use gst-launch-1.0 ... ! flacparse ! flacdec ! ....
it will still fail..

One thing i dont understand here is, in flacparse, the header is being set to
caps, which is being used in set_format, and the same header is again pushed to
baseparse, which means it is again being processed by the libflac.
Do we really need to handle the header twice?

> Now, *if* we want to support this broken file (because we know it's slightly
> broken), the solution should resides in flacparse. The parser should detect
> that the stream header has twice the stream info (hence the stream header is
> like two headers in one). Base on that, the parser could "fix" the header by
> only keeping the most recent one. Again, this is all about if we care
> supporting this broken file.

This file does not actually have two valid streaminfo. The first streaminfo is
valid. While the second is not valid(it just has type as streaminfo while
infact it is not) and it throws errors while parsing.
There is already code in flacparse to check this scenario and just ignore the
other parts of the header. This is the one set to streamheader and which
libflac is not able to process..
Now i did some simple experiments to determine what does libflac accept as a
valid metadata.
In case of a flac header, the end of header is determined by 
is_last = ((map.data[0] & 0x80) == 0x80)
so in the present case, since we are truncating the header and passing it to
libflac it fails.
Now when i try not to truncate and append all header till is_last is true, it
still fails because, there are two stream data of type streaminfo.

While parsing in flacparse, if i just remove the second part of streaminfo and
add everything else till is_last is true, to streamheader, this is being
accepted by libflac as a valid metadata. But the size of this stream header is
too big. 

But this fails when processing the actual frame :)... i guess this is because
the header does not end properly with is_last(0x80) value, and we still keep
searching in the actual data until we find something which we think is end of
header...


Not exactly sure if i am making any sense :)... too big a message!!!!
But the patch i attached now does not make any drastic change, other than give
libflac some more chances to prove itself, being worthy in good plugins :P

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.