One small step for ffmpeg... one giant leap for Emby! (Looking for C developer(s) to help with transcode throttling)

[FredipusRex 12]

Why this works: By turning off the paused_start state, we allow the input_thread loop to eventually call av_thread_message_queue_send. There is a flag inside that library that gets set when free_input_threads() is called, which is called when you have an AVERROR_EXIT result. That flag gets returned to the input_thread loop, which then shuts down the standard way.

 

ffmpeg has some code smells - for instance, when you quit the app, those input threads will do one final read even though the app is quitting. There are a couple of other instances of that sort of thing too. But we're not trying to fix ffmpeg - we're just trying to add pause/unpause.

[FredipusRex 12]

Ah, good catch! Okay, I'll make these changes and hopefully get it submitted later today...

 

Cool. Have you been able to test the pause/unpause capability within MB3? Does it do what is expected (save these final changes)?

[jluce50 118]

Cool. Have you been able to test the pause/unpause capability within MB3? Does it do what is expected (save these final changes)?

 

Not yet. I started working on building the toolchain that will allow me to cross-compile for Windows this morning. I'll pick it back up after I make these changes and get it submitted.

 

If I can manage to get a Windows build I'll probably send it to Luke and let him put out a dev build to test with.

 

Edit: If you're interested, this is what I'm attempting to use: http://ffmpeg.zeranoe.com/blog/?p=383#more-383

Edited March 9, 2015 by jluce50

[jluce50 118]

Okay, pull request submitted! Everyone cross your fingers and pray to whatever god(s) you believe in...

 

https://github.com/FFmpeg/FFmpeg/pull/124

 

I'll continue working on trying to get a Windows build, but I can't provide an ETA at this point.

[FredipusRex 12]

Looks like the developers don't actually use pull requests but work thru the ffmpeg-devel mailing list.

[jluce50 118]

Yeah, I saw that last night. I'll try to get it submitted to the mailing list today.

[jluce50 118]

I sent it in to the mailing list. I forgot to add [PATCH] to the subject, so hopefully I don't get reamed for that...

[jluce50 118]

Update on the Windows build. I managed to compile a bare-bones exe (no external libraries) and run it successfully on Windows. I just did a simple avi -> mpg conversion, but the pause functionality works and the output file plays fine.

 

The next step is to compile with all the libraries. This is going to be the hard part. There are a LOT of them (have a look at the External Libraries section here: http://ffmpeg.zeranoe.com/builds/). As far as I can tell, each one will need to be compiled for Windows from source and each one has potential for its own headaches. My guess is that my patch will be merged (assuming they accept it) and a build released by the time I would be able to get all this working. I'll keep plugging away, though...

[Luke 37090]

Zeranoe is fast so once merged you can expect a Windows build quickly

[FredipusRex 12]

Hmm, you might want to resubmit it with [PATCH] and do a clean diff. The diff you submitted was a concatenation of each edit you did (so some things are rolled back/overwritten by later diffs). You might want to submit a unified, single diff.

[jluce50 118]

Are you familiar enough with git that you could point me in the right direction? The only way I found to do that required that I hadn't pushed the commits back to the master, which I already had.

[FredipusRex 12]

You just need to diff between the last commit from before you cloned to the last commit on your branch. From your local repository, it should be:

 

git diff c285937ccc98109d0b5d4ba2e0560ebbb81f580f 036fededb42e66f1e45c34faa48519896fb56cc1

 

That first long ID is for unitux's "ffmpeg: comment mpeg4 hack".

 

The second long ID is for jluce50's "Add pausing for input threads and some cleanup"

 

(Usually you can get away with just the first 7 characters of each commit ID - git diff c285937 036fede - I'm just being paranoid.)

 

That should take care of it. Redirect the output to some file and check it out. If it looks good, use that.

 

(github's GUI will let you look at the history of each file, which is where I got the commit IDs, but doesn't have a way to generate a diff between two specific commits that I can see.)

Edited March 11, 2015 by FredipusRex

[FredipusRex 12]

Actually, there's a github hack to lets you generate a diff. If you enter your github repository URL but add /compare to the end (https://github.com/jluce50/FFmpeg/compare), you'll be brought to the Comparison page.

 

This URL will default to a comparison between where you forked (FFmpeg/FFmpeg/master) and your current master (jluce50/FFmpeg/master). This is actually what we want, so let's not mess with that. If you then go to your address bar and add ".diff" to the end of the URL - in this case:

https://github.com/FFmpeg/FFmpeg/compare/master...jluce50:master.diff

 

You'll get the plaintext diff:
 

diff --git a/ffmpeg.c b/ffmpeg.c
index 6604ff0..3498dfe 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -120,6 +120,9 @@ const char *const forced_keyframes_const_names[] = {
 static void do_video_stats(OutputStream *ost, int frame_size);
 static int64_t getutime(void);
 static int64_t getmaxrss(void);
+static int64_t gettime_relative_minus_pause(void);
+static void pause_transcoding(void);
+static void unpause_transcoding(void);
 
 static int run_as_daemon  = 0;
 static int nb_frames_dup = 0;
@@ -146,6 +149,9 @@ int         nb_output_files   = 0;
 FilterGraph **filtergraphs;
 int        nb_filtergraphs;
 
+int64_t paused_start = 0;
+int64_t paused_time = 0;
+
 #if HAVE_TERMIOS_H
 
 /* init terminal so that we can grab keys */
@@ -1447,7 +1453,6 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti
         last_time = cur_time;
     }
 
-
     oc = output_files[0]->ctx;
 
     total_size = avio_size(oc->pb);
@@ -3256,18 +3261,38 @@ static OutputStream *choose_output(void)
     return ost_min;
 }
 
+static void pause_transcoding(void)
+{
+    if (!paused_start)
+        paused_start = av_gettime_relative();
+}
+
+static void unpause_transcoding(void)
+{
+    if (paused_start) {
+        paused_time += av_gettime_relative() - paused_start;
+        paused_start = 0;
+    }
+}
+
 static int check_keyboard_interaction(int64_t cur_time)
 {
     int i, ret, key;
     static int64_t last_time;
-    if (received_nb_signals)
+    if (received_nb_signals) {
+        unpause_transcoding();
         return AVERROR_EXIT;
+    }
     /* read_key() returns 0 on EOF */
     if(cur_time - last_time >= 100000 && !run_as_daemon){
         key =  read_key();
         last_time = cur_time;
     }else
         key = -1;
+    // Reserve 'u' for unpausing a paused transcode, but allow any key to
+    // unpause for backward compatibility
+    if (key == 'u' || key != -1) unpause_transcoding();
+    if (key == 'p') pause_transcoding();
     if (key == 'q')
         return AVERROR_EXIT;
     if (key == '+') av_log_set_level(av_log_get_level()+10);
@@ -3346,7 +3371,9 @@ static int check_keyboard_interaction(int64_t cur_time)
                         "C      Send/Que command to all matching filters\n"
                         "D      cycle through available debug modes\n"
                         "h      dump packets/hex press to cycle through the 3 states\n"
+                        "p      pause transcoding\n"
                         "q      quit\n"
+                        "u      unpause transcoding\n"
                         "s      Show QP histogram\n"
         );
     }
@@ -3361,6 +3388,10 @@ static void *input_thread(void *arg)
     int ret = 0;
 
     while (1) {
+        if (paused_start) {
+            av_usleep(1000); // Be more responsive to unpausing than main thread
+            continue;
+        }
         AVPacket pkt;
         ret = av_read_frame(f->ctx, &pkt);
 
@@ -3778,6 +3809,11 @@ static int transcode_step(void)
     InputStream  *ist;
     int ret;
 
+    if (paused_start) {
+        av_usleep(10000);
+        return 0;
+    }
+
     ost = choose_output();
     if (!ost) {
         if (got_eagain()) {
@@ -3838,11 +3874,11 @@ static int transcode(void)
 #endif
 
     while (!received_sigterm) {
-        int64_t cur_time= av_gettime_relative();
+        int64_t cur_time = gettime_relative_minus_pause();
 
         /* if 'q' pressed, exits */
         if (stdin_interaction)
-            if (check_keyboard_interaction(cur_time) < 0)
+            if (check_keyboard_interaction(av_gettime_relative()) < 0)
                 break;
 
         /* check if there's any stream where output is still needed */
@@ -3885,7 +3921,7 @@ static int transcode(void)
     }
 
     /* dump report by using the first video and audio streams */
-    print_report(1, timer_start, av_gettime_relative());
+    print_report(1, timer_start, gettime_relative_minus_pause());
 
     /* close each encoder */
     for (i = 0; i < nb_output_streams; i++) {
@@ -3934,6 +3970,11 @@ static int transcode(void)
     return ret;
 }
 
+static int64_t gettime_relative_minus_pause(void)
+{
+    return av_gettime_relative() - paused_time -
+            (paused_start ? av_gettime_relative() - paused_start : 0);
+}
 
 static int64_t getutime(void)
 {

Edit: I changed pause/unpauseTranscode to pause/unpause_transcode in the above diff as ffmpeg never uses camelCase in their code. It'll give you away as a C# developer.

Edited March 11, 2015 by FredipusRex

[jluce50 118]

Ah, dammit! And I know better! I guess old habits die hard ;-)

 

I used git format-patch because that's what their docs say to use. I'll submit the diff tomorrow morning.

Edited March 11, 2015 by jluce50

[FredipusRex 12]

I'm pretty sure you can get the patch by adding .patch to the compare URL too, but I don't know how to do a unified patch as opposed to a patch per commit.

 

https://github.com/FFmpeg/FFmpeg/compare/master...jluce50:master.patch

 

Good luck!

[Luke 37090]

then why not just create a fresh fork, commit all into one changeset then create a path based on that?

[jluce50 118]

Actually, there's a github hack to lets you generate a diff. If you enter your github repository URL but add /compare to the end (https://github.com/jluce50/FFmpeg/compare), you'll be brought to the Comparison page.

 

This URL will default to a comparison between where you forked (FFmpeg/FFmpeg/master) and your current master (jluce50/FFmpeg/master). This is actually what we want, so let's not mess with that. If you then go to your address bar and add ".diff" to the end of the URL - in this case:

 

https://github.com/FFmpeg/FFmpeg/compare/master...jluce50:master.diff

 

You'll get the plaintext diff:

 

Adding ".patch" does actually work, but it's not unified. I'm compiling now just to make sure there aren't any name collisions with the new method names (there shouldn't be, but can't be too careful). That's how I discovered that pause() wouldn't work. 

 

Based on other submissions it looks like I can just paste this into my email rather than attaching the .diff or .patch file. And it looks like plenty of people are just using diffs instead of patches. Wish I would have realized all this the first time around...

[jluce50 118]

Okay, patch #2 sent. Hopefully I've got everything in order this time.

[jluce50 118]

Well, it's been 2 days and still no love. I'll send a follow-up message and see if I can get any bites. I really hope this isn't their way of rejecting the patch.

[FredipusRex 12]

Yeah, I could feel the crickets chirping there. Might also do a pull request again - someone seems to at least respond there, even if only to point to the ffmpeg-devel list. Another possibility is to email Michael Niedermeyer (sp?) who seems to be something of a gatekeeper.

[jluce50 118]

I was thinking of just adding a comment to the closed pull request. They should still get notified and hopefully respond. I thought about posting another pull request, but it seems like that might be crossing the line from being persistent to being a pest.

 

I'll give it another day or so and then email Mr. Niedermayer.

Edited March 13, 2015 by jluce50

[jluce50 118]

Finally got some attention. Looks like some line breaks got introduced somehow (Gmail?). I'll just save it to a diff file and attach that next time.

 

He also wants me to update the documentation, but I don't see the keyboard interaction documented anywhere but in the source code. Looking at the doc for the main ffmpeg tool, I don't see an appropriate place for this info (maybe under "Tips"?). Any suggestions?

[Luke 37090]

look for the 'q' key in the documentation and put it there. if you can't find that, ask him. good sign though

[Luke 37090]

just to confirm - what are the keys i should use to turn on/off?

 

p and u, correct?

 

i think it is safe for me to add since right now it will just do nothing and not cause any problems.

[jluce50 118]

That's correct