patchGnash - The GNU Flash player - Patches: patch #7585, Removing parse parameter from...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #7585: Removing parse parameter from AudioDecoder interface

Submitter:  Jonas 'Sortie' Termansen <sortie>
Submitted:  Wed 27 Jul 2011 07:45:23 PM UTC
   
 
Category:  sound Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  strk Open/Closed:  Closed

Jump to the original submission

Thu 28 Jul 2011 03:39:15 PM UTC, comment #7: 
Sandro Santilli <strk>
Group Member
Thu 28 Jul 2011 02:28:55 PM UTC, comment #6: 

I reuploaded the patch rebased on master.

(file #23711)

Jonas 'Sortie' Termansen <sortie>
Group Member
Thu 28 Jul 2011 02:12:04 PM UTC, comment #5: 

FFmpeg version SVN-r0.5.1-4:0.5.1-1ubuntu1.1

Could you upload a patch applying cleanly to master ?

Sandro Santilli <strk>
Group Member
Thu 28 Jul 2011 11:20:42 AM UTC, comment #4: 

I very carefully checked every implementation of the decode() function to see that the parameter was used. Only AudioDecoderFfmpeg used it. Thus, any other backends will work as usual.

AudioDecoderFfmpeg only used the parameter to log errors if it didn't match the _needParsing member variable. This means the parse parameter represents something AudioDecoderFfmpeg already knows, and thus removing it only gets rid of the errors. It is therefore safe to remove the parameter entirely.

I assume that if no parser is constructed - and one is needed - then the actual decoding will fail. Otherwise no parser was needed in the first place. We can't determine whether a parser is needed until we try to decode something without one.


You have no vorbis files when running my testcase? You probably have an older version of ffmpeg(1) or libvorbis?

Jonas 'Sortie' Termansen <sortie>
Group Member
Thu 28 Jul 2011 09:22:37 AM UTC, comment #3: 

Note that I get a single XPASS with your tests:

XPASS: test_decoding: ffmpeg: samples/sample-pcm.wav

no unexpected failures in FFMPEG (only those known failures in GST, including the abort).

But indeed I've no vorbis files..

Sandro Santilli <strk>
Group Member
Thu 28 Jul 2011 09:05:26 AM UTC, comment #2: 

I've seen that you consider a failing parser constructor as the sign of having no need to parse... but how do you distinguish between no-need and an actual failure ?

Sandro Santilli <strk>
Group Member
Thu 28 Jul 2011 08:22:45 AM UTC, comment #1: 

Was this tested with both embedded and external sounds ?
Do we have automated tests for both cases ?

Sandro Santilli <strk>
Group Member
Wed 27 Jul 2011 07:45:23 PM UTC, original submission:  

The parse parameter used in the "uint8_t* decode(uint8_t* input, uint32_t inputSize, uint32_t& outputSize, uint32_t& decodedData, bool parse)" function in AudioDecoder is not needed.

Every backend but FFmpeg ignores it, and FFmpeg only uses it to log error messages if the caller guesses the wrong value - the AudioDecoder class figures the need automatically.

I went ahead and removed it because it is unnecessary, and that the system actually creates problems in my upcoming libmedia testcase. Internally, AudioDecoderFfmpeg requires a parser for any non-flash codec, but most does not require (or even have) one. This prevents that class from loading these media files.

In this patch, I remove the parameter, and change AudioDecoderFfmpeg to attempt creating a parser, and if not possible, carry on regardleses. This behavior fixes ffmpeg playback of 3 of my sample files (sample-vorbis.mkv, sample-vorbis.webm, sample-pcm.wav).

Jonas 'Sortie' Termansen <sortie>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by strk (Posted a comment)
  • -email is unavailable- added by sortie (Submitted the item)
  • -email is unavailable- added by sortie
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-07-28 strk StatusNone Done
        Assigned toNone strk
        Open/ClosedOpen Closed
    2011-07-28 sortie Attached File- Added 0001-Removed-parse-parameter-from-AudioDecoder-interface.patch, #23711
    2011-07-28 sortie Attached File#23704 Removed
    2011-07-27 sortie Attached File- Added 0001-Removed-parse-parameter-from-AudioDecoder-interface.patch, #23704
        Carbon-Copy- Added strk

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code