bugGnash - The GNU Flash player - Bugs: bug #39848, gui/fb/fb.cpp:382:9: warning:...

 
 

bug #39848: gui/fb/fb.cpp:382:9: warning: variable 'delay' set but not used

Submitted by:  Gabriele Giacone <gg0>
Submitted on:  Thu 22 Aug 2013 01:40:29 AM UTC  
 
Category: gui-fbSeverity: 2 - Minor
Release: NoneStatus: Ready For Test
Privacy: PublicAssigned to: Rob Savoye <rsavoye>
Open/Closed: Open

Add a New Comment (Rich MarkupRich 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)

Thu 31 Oct 2013 07:21:55 PM UTC, comment #6:

Link to Rob's commit, for easy access:
http://git.savannah.gnu.org/cgit/gnash.git/commit/?id=bc65a35d1b4a5652ae1e0a9418a35093cdc9c527

Sandro Santilli <strk>
Project Member
Sat 26 Oct 2013 08:13:11 PM UTC, comment #5:

Gabriele please close if you're happy with this (the warning thing).

Rob: I'm just saying that the delay control should be done with commandline switches. Note that _interval is meant to control the delay, and the multiplication by 1000 is because gnashSleep takes nanoseconds while _interval is expressed in milliseconds.

The "delay" variable is really a (bad, imho) hack.
If you want to override heartbeat frequency you should act on _iterval, not the constant factor between nanoseconds and microseconds.

Sandro Santilli <strk>
Project Member
Sat 26 Oct 2013 07:50:29 PM UTC, comment #4:

So this delay thing is very useful on very slow targets where Gnash is beibg used as the UI, and nit as a browser plugin nor media player. For those platforms, this delay is very useful. That said, there aren't that many people using Gnash this way anymore, so maybe it is meaningless at this point. At the same time it's important to remember that Gnash is used in many different ways.

A side note is the fb GUI doesn't work anyway, as we have no way to calibrate the screen. Anyway, I just reenabled the delay factor with #bc65a35.

Rob Savoye <rsavoye>
Project AdministratorIn charge of this item.
Sat 26 Oct 2013 01:58:59 PM UTC, comment #3:

The delay variable was introduced by Rob to reduce CPU load by reducing the heartbeat for SWF apps having a nominal framerate equal or minor to 12 (a common default). This entered the codebase with this commit: http://git.savannah.gnu.org/cgit/gnash.git/commit/gui/fb/fb.cpp?id=3bf5e4607b866c336dc4471facdd1c410c8d0e51

But the same support was later (accidentally, I guess?) dropped by himself with this unrelated commit: http://git.savannah.gnu.org/cgit/gnash.git/commit/gui/fb/fb.cpp?id=a7af918e32a9bafbe2d545ec591a550d3e430a72

Personally I think the the delay should always be 10ms as before the first commit (it's requried to be compatible with timer resolution, fps of loaded movies and/or media, thus not only depending on nominal SWF framerate) and that CPU load reduction should be done by using a generic switch to control heartbeat (giving user the possibility to giveup some compatibility to relax CPU use).

So my vote on this is to just drop the "delay" variable.

Sandro Santilli <strk>
Project Member
Sun 29 Sep 2013 02:46:57 PM UTC, comment #2:

It's not used by the current code, as compiler reports.
The commented-out use is in a log_debug, nothing more I can see.

Sandro Santilli <strk>
Project Member
Thu 22 Aug 2013 03:08:30 PM UTC, comment #1:

This is a touch bogus, as delay is used, although currently it's commented out. (but shouldn't be) Some flow framebuffer only targets need this feature.

Rob Savoye <rsavoye>
Project AdministratorIn charge of this item.
Thu 22 Aug 2013 01:40:29 AM UTC, original submission:

introduced delay with 3bf5e460 then replaced with 1000 back with a7af918e3.
Getting rid of it completely or using it?

Gabriele Giacone <gg0>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by strk (Posted a comment)
  • -unavailable- added by rsavoye (Updated the item)
  • -unavailable- added by gg0 (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 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 26 Oct 2013 08:13:11 PM UTCstrkStatusWorks For Me=>Ready For Test
    Sat 26 Oct 2013 07:50:29 PM UTCrsavoyeStatusConfirmed=>Works For Me
    Sat 26 Oct 2013 02:14:57 PM UTCstrkCategoryNone=>gui-fb
    Sat 26 Oct 2013 01:58:59 PM UTCstrkStatusWorks For Me=>Confirmed
    Thu 22 Aug 2013 03:08:30 PM UTCrsavoyeStatusNone=>Works For Me

    Back to the top


    Powered by Savane 3.1-cleanup