Re: [Gnash-dev] Summer of Code Status

[Sandro Santilli]

On Wed, Aug 24, 2011 at 11:40:31AM +0200, Jonas 'Sortie' Termansen wrote:

> The AudioDecoder interface is not designed such that the input may
> be reused later. This is critical for the libavcodec backend, which
> expects us to re-provide the same data packet, if the data packet
> contained multiple audio frames. To solve this, calling the decode()
> function transfers ownership of the data packet to the AudioDecoder,
> which may then store the data in a queue until it has been used. In
> effect, this implements the push/pop mechanism we intended
> AudioDecoder to have.

There was a ticket about the push/pop mechanism. Can you find it ?
I don't see any documentation about ownership of buffer passed
to AudioDecoder in AudioDecoder.h. There should be.

> Another critical design flaw is that MediaParser has no way to
> signal whether a file is not supported because 1) the codec is
> missing 2) the file is broken 3) our code is broken. Because of
> this, my testcases assume the worst and declare the file FAILed.

We can't really tell if the file or our code is broken.
Also, a missing codec _is_ a FAILure to me. Due to setup rather
than code quality, but still a failure, so it's ok to report it
as such. 

> This has caused me problems on our buildbots, where the needed
> codecs weren't installed. gg0 addressed this by installing most of
> the codecs, which means most of the testcases now succeed. However,
> the testcases still vary greatly depending on the system
> configuration and library versions, instead of varying depending on
> whether our code is actually broken.

As long as our usual build bots and the developers machines have
successes we're fine with it. System setups affecting testsuite will
also affect normal run experience, so is ok to report failures when
encountered. Eventually, you can drop tests for formats/codecs which
are not particularly interesting to test (not supported by the official
player and not open formats).

> MediaParser also has a lot of issues, such as if something went
> wrong while parsing the file, it cannot signal that something went
> wrong. Rather, it does nothing and my testcase thread must timeout
> after 2 seconds and warn that something _might_ have gone wrong.

That's fine to me.
In a normal run we don't want to stop the world on a decoding error.
You could get finer reporting by having the MediaParser throw an
exception and catch it both by in-player clients and your testcase
but most of the times testcases that pretend to be the player are
more interesting (easier to maintain and easier to feed to the official
player for comparison) so I wouldn't bother.

> I've been hesitant to push my testcases because of these issues.
> While useful, my testcases would do nothing but spam spurious FAILes
> or XPASSes when running on the buildbots. I would like to address
> the MediaParser issues before pushing my testcases to git master.
> (If anyone wants to review my testcase code and other changes, tell
> me and I'll give it to you.)

I think the testcases are the most important part of your work.
Please put the code somewhere accessible 24/7 so we can test them
and report unexpected results. I'm assuming you'll test it against
the buildbot.

> My testcases needs a lot of fixes in libmedia to work, and I have
> already addressed quite a few of them, most notably multi-packet
> decoding, proper codec detection in AudioDecoderFfmpeg, and more. I
> have already pushed some of these fixes, but the multipacket support
> is not pushed yet. This is simply because my testbuild has been
> overly crashy when running Youtube - I want to be sure I don't cause
> random crashes. :)

Mixing testcase work and gnash fixes is a dangerous thing.
That's what XFAIL is for (accept the failure while focusing
on the testcase). 

> So what now?
> 
> I expect to keep working on Gnash. First, I'll bugcheck my
> multiframe code and then push it to git master. Then I'll take on
> the task of reworking MediaParser, such that it can communicate
> failures better. Hopefully, this should make fix the last few
> failing testcases so I can push it.
> 
> Then I want to move media decoding to the media parsing thread. This
> offers performance benefits for libavcodec, which prefers if we
> decode a packet before we parse the next (and the previous packet is
> garbage collected). In addition, it breaks the 'one-packet
> one-frame' paradigm which the current AudioDecoder interface suffers
> from. Lastly, MediaParser will work as intended: you seek to a
> position in the file, and it returns a decoded audio/video frames
> with associated metainformation (timestamp, etc.).
> 
> 
> So overall, most of my code has been pushed to git master, some of
> it isn't because it requires fixes in libmedia. I intend to perform
> these fixes over the coming weeks, such that my testcases can be
> pushed soon. Then I'll continue to make improvements to libmedia.
> 
> Thanks for a great summer!

Thank you for the hard work so far.
Looking forward to see your testcases (and, only secondarily, fixes ;)
pushed !

--strk;

  ()   Free GIS & Flash consultant/developer
  /\   http://strk.keybit.net/services.html