Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] FFMPEG bug causes Obico not to respect RFC2396 for stream URLs causing compatible mode to run at 0.1 fps #174

Open
puterboy opened this issue Jun 24, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@puterboy
Copy link
Contributor

In Octoprint settings, under "Webcam & Timelapse", if you set stream url to:
http://myrpi:8080/?action=stream which is a valid RFC2396 URL, then ffmpeg will fail to recognize the URL and returns:
http://myrpi:8080?action=stream: Server returned 400 Bad Request

This is actually an ffmpeg bug
See the bug report: https://trac.ffmpeg.org/ticket/8466

Obico doesn't detect this bug and merrily launches ffmpeg with the faulty URL.
This causes ffmpeg fails (silently) and then Obico defaults to 0.1fps snapshots.

Suggested fix is either:

  1. Warn users if 'malformed' URL (and document to prevent users like me from wasting hours of time and having to debug the code to figure out what is going on)
  2. Or add a manual fix to stream_url in webcam_stream.py to insert a slash if missing before '?'

Of course, until ffmpeg or Obico code is adjusted, users can avoid the problem by making sure there is a "/" before the '?'.
i.e., change http://myrpi:8080?action=stream to http://myrpi:8080/?action=stream

@kennethjiang
Copy link
Contributor

Good catch. Feel free to send a PR for it.

@kennethjiang kennethjiang added the bug Something isn't working label Jun 25, 2022
@puterboy
Copy link
Contributor Author

puterboy commented Jun 27, 2022

I'm not really a Python programmer, but I think this should work (feel free to correct my crude "style")

--- webcam_stream.py    2022-06-26 22:23:20.631896668 -0400
+++ webcam_stream.py.new   2022-06-26 22:30:05.722292999 -0400
@@ -1,5 +1,6 @@
 import io
 import re
+from urllib.parse import urlparse
 import os
 import logging
 import subprocess
@@ -192,6 +193,9 @@
         jpg = wait_for_webcamd(webcam_settings)
         (_, img_w, img_h) = get_image_info(jpg)
         stream_url = webcam_full_url(webcam_settings.get("stream", "/webcam/?action=stream"))
+        stream_url_parse = urlparse(stream_url)
+        if stream_url_parse.query and not stream_url_parse.path :
+            stream_url = stream_url_parse._replace(path='/').geturl()
         bitrate = bitrate_for_dim(img_w, img_h)
         fps = 25
         if not self.plugin.is_pro_user():

@puterboy
Copy link
Contributor Author

puterboy commented Jun 27, 2022

Alternatively, you could more broadly (and cleanly) add the patch to the definition of webcam_full_url in webcam_capture.py
Something like:

def webcam_full_url(url):
    if not url or not url.strip():
        return None

    full_url = url.strip()
- if not urlparse(full_url).scheme:
+ url_parse=urlparse(full_url)
+ if not url_parse.scheme
        full_url = "http://localhost/" + re.sub(r"^\/", "", full_url)
+ elif url_parse.query and not url_parse.path :
+     full_url = url_parse._replace(path='/').geturl()    

    return full_url

@puterboy
Copy link
Contributor Author

Any reason this patch (or similar) has not been included in latest updates?

@kennethjiang
Copy link
Contributor

It'll be a lot easier for me if you can submit a PR so that we can discuss the specific code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants