bugGnash - The GNU Flash player - Bugs: bug #46607, Embedded audio quality degradation

 
 

bug #46607: Embedded audio quality degradation

Submitted by:  Sandro Santilli <strk>
Submitted on:  Fri 04 Dec 2015 10:44:34 PM UTC  
 
Category: soundSeverity: 5 - Blocker
Release: masterStatus: Fixed
Privacy: PublicAssigned to: Sandro Santilli <strk>
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Wed 16 Dec 2015 10:19:32 AM UTC, comment #19:

I think the audio degradation issue is gone, so closing.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Wed 16 Dec 2015 08:53:27 AM UTC, comment #18:

Filed another ticket for the missing sound: #46693

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 15 Dec 2015 05:04:17 PM UTC, comment #17:

I've found another regression in sound handling, namely sounds that are missing from output. I'm doing further research.
Commit b7a877c works fine, so it must be another one after that.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 08 Dec 2015 11:57:17 AM UTC, comment #16:

On the other hand, in the tennis.swf test, Bastiaan code correctly emits the "bouncing tennis ball" sounds when hovering on the Play/Practice buttons, while current master does not.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 08 Dec 2015 11:44:33 AM UTC, comment #15:

I found another test showing a difference of behavior with Bastiaan code and w/out:
http://strk.keybit.net/tmp/tennis.swf

The music at start is supposed to continue/loop while it stops shortly after starting with Bastiaan code (as patched with my patch).

Maybe it's not really worth reverting the revert until we can perform more tests...

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 08 Dec 2015 11:25:26 AM UTC, comment #14:

Sorry for the naming, the method patched is "consumeInPoint", not "consumeInput".

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 08 Dec 2015 11:24:24 AM UTC, comment #13:

I've looked closely at bastiann code and found the problem being in the LiveSound::consumeInput function. I have a patch that fixes that implementation so we might go back to the new implementation, but would like a review from Bastiaan.

The patch applies to the code before the revert.
I'm attaching it here (consumeInput.patch).

Basically the "delaySeek" value is larger than the size of he first buffer, so setting _consumed to the requested size is incorrect, and leaves _pos in a wrong state.

Bastiaan, if you think the new code is worth it, I'd revert the revert and apply this patch. I dunno if you run performance tests to check how the new code compared to the old...

(file #35661)

Sandro Santilli <strk>
Project MemberIn charge of this item.
Mon 07 Dec 2015 05:45:36 PM UTC, comment #12:

I found a shareable example of a streaming mp3 sound showing the degradation. Not very focused but it's a start.

Alfy Havoc: http://strk.keybit.net/tmp/alfy-havoc.swf

Start the video, click on the coo-coo clock when asked to
and hear to the phrase pronounced by Alfy:

"It's 3 o'clock, let's go get my puppy"

It sounds badly with eda27746fcd190fc4092e0cb1166726bebf000df (before reverting bastiaan change) and it is fixed by the revert in commit 41ae739439c796f7f310f813f18cc12287b2e450

Sandro Santilli <strk>
Project MemberIn charge of this item.
Mon 07 Dec 2015 04:42:49 PM UTC, comment #11:

@strk's follow-ups on this issue somehow ended up in patch #8721,
I took a liberty to quote them here for continuity:

Comment 12:

Comment 13:

Nutchanon Wetchasit <nachanon>
Mon 07 Dec 2015 03:18:10 PM UTC, comment #10:

latencySeek in the SoundStreamHead seem to make a difference between playing nicely (with no latencySeek) and badly (w/out).
This is when NOT reverting bastiaan commit.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Mon 07 Dec 2015 02:45:02 PM UTC, comment #9:

reverted with 41ae739439c796f7f310f813f18cc12287b2e450 -- I'm still working on a reduced testcase (not automated).

Sandro Santilli <strk>
Project MemberIn charge of this item.
Sat 05 Dec 2015 04:24:27 PM UTC, comment #8:

No, go ahead.

Bastiaan Jacques <bjacques>
Project Member
Sat 05 Dec 2015 04:14:16 PM UTC, comment #7:

I tried a revert and it does fix the quality issue.
Will now look for a way to provide a simple testcase.
Bastiaan, do you have any objection about reverting that commit ?

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri 04 Dec 2015 11:04:54 PM UTC, comment #6:

Both gstreamer and ffmpeg media handlers are affected (as expected)

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri 04 Dec 2015 11:03:45 PM UTC, comment #5:

Ok, I do can confirm. Sorry for the double-fail, too many build directories around.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri 04 Dec 2015 11:02:30 PM UTC, comment #4:

BUT, I cannot confirm this commit was really the culprit :/

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri 04 Dec 2015 11:01:40 PM UTC, comment #3:

I think a "realloc" should reduce the pressure by a huge degree, if the problam was the new allocation and the copy. Internally, the virtual memory subsystem would just do what you attempted to do via C++ code.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri 04 Dec 2015 10:55:38 PM UTC, comment #2:

The embedded sound is an mp3, single HEAD and 223 BLOCKS.

Block type: 18 (SWF_SOUNDSTREAMHEAD)
Block length: 6

PlaybackSoundRate 44 kHz
PlaybackSoundSize 16 bit
PlaybackSoundType mono
StreamSoundCompression MP3
StreamSoundRate 44 kHz
StreamSoundSize 16 bit
StreamSoundType mono
StreamSoundSampleCount 1837
LatencySeek 1633

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri 04 Dec 2015 10:44:59 PM UTC, comment #1:

This is actually a blocker, being a regression

Sandro Santilli <strk>
Project MemberIn charge of this item.
Fri 04 Dec 2015 10:44:34 PM UTC, original submission:

I've found a movie which plays stuttering and bisecting got to this point:

a9f7f743c66f2f36d81c348da238d94877473372 is the first bad commit
commit a9f7f743c66f2f36d81c348da238d94877473372
Author: Bastiaan Jacques <bastiaan@bjacques.org>
Date: Sun Jun 15 20:09:12 2014 +0200

Reduce malloc() churn for SWF-embedded audio.

Decoded data from audio sources stored inside SWFs are decoded and
stored in a growing buffer. This buffer is appended to as small
chunks of data become available. In order to store the additional
data, a new buffer big enough to contain all the data is allocated.
A minute of audio corresponds to about 10MB of decoded data. The
ensuing (re)allocation of such a large buffer adds significantly to
the resource pressure on the system.

This change alters the situation so that instead of allocating a new
buffer, each buffer is added to a list as-is, at the cost of having
to keep track of reads in more detail.

:040000 040000 45eed7e653f151eeaf7baf2187d08887b6c169c5 c61e4016939188ae645ff56a9d7a9c972fb241d8 M libbase
:040000 040000 c5a5e6700aac7e4a084a4fe3fe463469c4cde316 5a86380f1b008ecc77da09db22b10e3bcd61c71e M libsound

Sandro Santilli <strk>
Project MemberIn charge of this item.

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #35661:  consumeInput.patch added by strk (1001B - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by nachanon (Posted a comment)
  • -unavailable- added by bjacques (Posted a comment)
  • -unavailable- added by strk (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 16 Dec 2015 10:19:32 AM UTCstrkStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Tue 08 Dec 2015 11:24:24 AM UTCstrkAttached File-=>Added consumeInput.patch, #35661
    Mon 07 Dec 2015 02:45:02 PM UTCstrkStatusNone=>Ready For Test
      Assigned tobjacques=>strk
    Fri 04 Dec 2015 10:44:59 PM UTCstrkSeverity3 - Normal=>5 - Blocker

    Back to the top


    Powered by Savane 3.1-cleanup1