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 Aug 22 01:40:29 2013  
 
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 Oct 31 19:21:55 2013, 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 Oct 26 20:13:11 2013, 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 Oct 26 19:50:29 2013, 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 Oct 26 13:58:59 2013, 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 Sep 29 14:46:57 2013, 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 Aug 22 15:08:30 2013, 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 Aug 22 01:40:29 2013, 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 Oct 26 20:13:11 2013strkStatusWorks For Me=>Ready For Test
    Sat Oct 26 19:50:29 2013rsavoyeStatusConfirmed=>Works For Me
    Sat Oct 26 14:14:57 2013strkCategoryNone=>gui-fb
    Sat Oct 26 13:58:59 2013strkStatusWorks For Me=>Confirmed
    Thu Aug 22 15:08:30 2013rsavoyeStatusNone=>Works For Me

    Back to the top


    Powered by Savane 3.1-cleanup