Looking for Perl Master to fix AcidRip's progress output

[wyohman]

I think the title says it all. There's a bug in AcidRip which causes the status window to display no status. Somewhere around line 866 is the Perl regexp that needs to be corrected. I've find one solution on the Internet but it only fixes one part. Any help would be appreciated.

Cheers.

[travisN000]

I sent a PM to don_crissti on the gnome forums..   I think he may be able to help

[wyohman]

Awesome. Thanks.

Cheers.

[travisN000]

Just for clarification, is this the code you are trying to patch:

sub encode {

  #fork proofing
  open STDIN, '/dev/null' or message("Can't read /dev/null: $!. May fail if in background");

  my $menc = shift;

  if ( defined $::widgets ) {
    my $buffer = $::widgets->{'mencoder_output_text'}->get_buffer;
    my ( $sec, $fps, $size, $time, $rate, $prog ) = ( 0, 0, 0, 0, 0, -1 );
      my $this_event = $::settings->{'total_events'} - queued_encode_events();
  
    if ( $::settings->{'mencoder_pid'} = open( MENCODER, "$menc 2>&1 |" ) ) {
      $/ = "\r";
         while (<MENCODER>) {
       if (/^Pos:\s*(\d+).\ds\s+(\d+)f\s+\(\s*(\d+)%\)\s+(\d+fps)\sTrem:\s+(\d+min)\s+(\d+mb).+\[([\d:]+)\]/) {
          if ( $1 ne $sec )  { $sec  = $1; $::widgets->{'menc_seconds'}->set_text( hhmmss($1) ) }
          if ( $4 ne $fps )  { $fps  = $4; $::widgets->{'menc_fps'}->set_text($4) }
          if ( $6 ne $size ) { $size = $6; $::widgets->{'menc_filesize'}->set_text($6) }
          if ( $5 ne $time ) { $time = $5; $::widgets->{'menc_time'}->set_text($5) }
          if ( $7 ne $rate ) { $rate = $7; $::widgets->{'menc_bitrate'}->set_text($7) }
          if ( $3 ne $prog ) {
                  $prog = $3;
                  $::widgets->{'menc_progress'}->set_fraction( $3 / 100 );
                my $progress = sprintf("%d", ($this_event - 1 + ($3/100))*100/$::settings->{'total_events'});
                $::widgets->{'acidrip'}->set_title(msg("acidrip") . ' - ' . $progress . '%');
                  $::widgets->{'progress_dialog'}->set_title(msg("acidrip") . ' - ' . $progress . '%');
               }
        }
        else { s/\r/\n/g; $buffer->insert_with_tags_by_name( $buffer->get_end_iter, $_, 'mplayer' ) }
        while ( Gtk2->events_pending() ) { Gtk2->main_iteration() }
      }
      $/ = "\n";
    }
    else { message( "Mencoder failed, is it properly installed?", 'error' ) }
  }
  else {
    message( "Mencoder failed, is it properly installed?", 'error' ) if !( $::settings->{'mencoder_pid'} = open( MENCODER, "$menc" ) );
  }
  $::widgets->{'view_button'}->set_sensitive( -r ( substitute_filename( $::settings->{'filename'} . ( $::settings->{'mpegfile'} ? ".mpg" : ".avi" ) ) ) ? 1 : 0 );
  close MENCODER;
}

 

..also, what was the partial fix you found?

My Guess it that a capture of some of the mencoder output may help as well, so that whoever tackles this knows the structure of the input that AcidRip / perl is actually processing from mencoder (..save them the work if possible).

[wyohman]

Just for clarification, is this the code you are trying to patch:

if (/^Pos:\s*(\d+).\ds\s+(\d+)f\s+\(\s*(\d+)%\)\s+(\d+fps)\sTrem:\s+(\d+min)\s+(\d+mb).+\[([\d:]+)\]/) {

 

..also, what was the partial fix you found?

My Guess it that a capture of some of the mencoder output may help as well, so that whoever tackles this knows the structure of the input that AcidRip / perl is actually processing from mencoder (..save them the work if possible).

Specifically line 866. Here's the replacement code I found:

if (/^Pos:\s*(\d+)\.\ds\s+(\d+)f\s+\(\s*(\d+)%\)\s+([\d.]+fps)\sTrem:\s+(\d+min)\s+(\d+mb).+\[([\d:]+)\]/) {

The change is very subtle but at least gives some output. However most of the process is not updated. It looks like Acidrip is essentially unsupported at this point.

You're right about it parsing the output of mencoder. It looks like this:

Pos:  54.9s   1648f ( 2%) 178.72fps Trem:   6min 1596mb  A-V:0.054 [5151:192]

Cheers.

[travisN000]

if (/^Pos:\s*(\d+).\ds\s+(\d+)f\s+\(\s*(\d+)%\)\s+(\d+fps)\sTrem:\s+(\d+min)\s+(\d+mb).+\[([\d:]+)\]/) {

if (/^Pos:\s*(\d+)\.\ds\s+(\d+)f\s+\(\s*(\d+)%\)\s+([\d.]+fps)\sTrem:\s+(\d+min)\s+(\d+mb).+\[([\d:]+)\]/) {

The change is very subtle but at least gives some output. However most of the process is not updated. It looks like Acidrip is essentially unsupported at this point.

You're right about it parsing the output of mencoder. It looks like this:

Pos:  54.9s   1648f ( 2%) 178.72fps Trem:   6min 1596mb  A-V:0.054 [5151:192]

Cheers.

..seems like quite the puzzle!   ..I highlighted the differences above; I'm not much for regex's, but I'm going to try and see if I can break it down with the help of google.
 

[travisN000]

What parts of AcidRip's progress are updated, and which are not?

[wyohman]

What parts of AcidRip's progress are updated, and which are not?

Time encoded is the only field updated.

Cheers.

[travisN000]

This is just a guess, but if the modified regexp that you posted is able to display elapsed time from mencoders output (..it is), then I think the regexp is no longer the problem...  that is, setting of the elapsed time only happens if the output of mencoder matches the regex  (line 867 only happens if line 866 evaluates to true).

This means that the remaining problems lie elsewhere..  ..most likely in the handful of lines following.  I think the issue is just that the $4, $6, etc numbers are wrong.  I'm going to modify these and see if I can get it working with your regex.

[daniel]

sent fixed version...

[travisN000]

Out of curiosity, what was your fix?

It seems that the if-then statements in the lines following the regex are the issue...  If they are modified so that the GUI widgets are updated every time the regex is matched, then all progress info seems to work...  but I think the if-then statements below the regex were added to limit the number of updates to the GUI widgets (less resources used?).  I'm just not sure why the if-then statements in the original code don't work as expected (I'm not a perl programmer).


for example, the following modifications do work (because $0 will never equal $sec, $fps, etc):

       if (/^Pos:\s*(\d+)\.\ds\s+(\d+)f\s+\(\s*(\d+)%\)\s+([\d.]+fps)\sTrem:\s+(\d+min)\s+(\d+mb).+\[([\d:]+)\]/) {
          if ( $0 ne $sec )  { $sec  = $1; $::widgets->{'menc_seconds'}->set_text( hhmmss($1) ) }
          if ( $0 ne $fps )  { $fps  = $4; $::widgets->{'menc_fps'}->set_text($4) }
          if ( $0 ne $size ) { $size = $6; $::widgets->{'menc_filesize'}->set_text($6) }
          if ( $0 ne $time ) { $time = $5; $::widgets->{'menc_time'}->set_text($5) }
          if ( $0 ne $rate ) { $rate = $7; $::widgets->{'menc_bitrate'}->set_text($7) }

[daniel]

for me the progress_output, don't work anyway. Now the Timecode works...
But something is change in mencoder, i think, there works the timecode not 

[travisN000]

try the changes in my previous post.. 

[daniel]

will do that tomorrow, now it's time to go to bed  
ps. i removed the update for the moment  

[wyohman]

It seems that the if-then statements in the lines following the regex are the issue...  If they are modified so that the GUI widgets are updated every time the regex is matched, then all progress info seems to work...  but I think the if-then statements below the regex were added to limit the number of updates to the GUI widgets (less resources used?).  I'm just not sure why the if-then statements in the original code don't work as expected (I'm not a perl programmer).

Good catch. Of course the IF is working otherwise we'd get no updates! DUH!

for example, the following modifications do work (because $0 will never equal $sec, $fps, etc):

      if (/^Pos:\s*(\d+)\.\ds\s+(\d+)f\s+\(\s*(\d+)%\)\s+([\d.]+fps)\sTrem:\s+(\d+min)\s+(\d+mb).+\[([\d:]+)\]/) {[/color][/b]
          if ( $0 ne $sec )  { $sec  = $1; $::widgets->{'menc_seconds'}->set_text( hhmmss($1) ) }
          if ( $0 ne $fps )  { $fps  = $4; $::widgets->{'menc_fps'}->set_text($4) }
          if ( $0 ne $size ) { $size = $6; $::widgets->{'menc_filesize'}->set_text($6) }
          if ( $0 ne $time ) { $time = $5; $::widgets->{'menc_time'}->set_text($5) }
          if ( $0 ne $rate ) { $rate = $7; $::widgets->{'menc_bitrate'}->set_text($7) }

I believe the nested IFs are used to update the data ONLY is there is a change.

In red we evaluate the nested IF and if the current value ($0 or whatever) is not equal to the previous value update it for next comparison and then update the screen.

So it appears there's something not right about the variables. Since the data gets updated properly if you make the IF FALSE, I'm at a bit of a loss.

Thanks for the second set of eyes. Maybe we'll get pointed in the right direction.

Cheers.