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

Source.probe sends file:// URIs to ffprobe as a ReadableStream, degrading data quality #13

Closed
agrathwohl opened this issue Feb 12, 2020 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@agrathwohl
Copy link
Contributor

Currently, all file:// URIs appear to be sent to ffprobe as a ReadableStream rather than a local file path string. Though a necessary step when wishing to send anything other than a local file to ffprobe, this degrades the quality of data we can collect at the container level when the file is local.

little-media-box/source.js

Lines 221 to 234 in febdee8

this.ready((err) => {
if (err) { return callback(err) }
const { uri } = this
const stream = this.createReadStream(opts)
this.active()
ffmpeg(stream).ffprobe((err, info) => {
this.inactive()
info.format.filename = info.format.filename === 'pipe:0' ?
path.basename(this.uri) :
info.format.filename
callback(err, info)
})
})

The above code provides an 'N/A' value for info.format.duration, info.format.bit_rate, info.format.size, and others. This has an effect on the probe_score value, which is a very important stat returned by ffprobe.

If the URI is a file:// address, and the following code change is made, all keys within info.format present correct values.

    this.ready((err) => {
      if (err) { return callback(err) }
      /*
      const { uri } = this
      const stream = this.createReadStream(opts)
      */

      this.active()
      ffmpeg(this.uri).ffprobe((err, info) => {
        this.inactive()
        info.format.filename = info.format.filename === 'pipe:0' ?
          path.basename(this.uri) :
          info.format.filename
        callback(err, info)
      })
    })
@bcomnes
Copy link
Contributor

bcomnes commented Feb 12, 2020

So ffmpeg can take a stream or a file URI?

@agrathwohl
Copy link
Contributor Author

agrathwohl commented Feb 12, 2020

fluent-ffmpeg offers the ability to provide:

  • one ReadableStream
  • one or more file URIs

as input into ffmpeg. ffprobe specifically may take one and only one input, of either a stream or a file URI. The documentation states:

Warning: ffprobe may be called with an input stream, but in this case it will consume data from the stream, and this data will no longer be available for ffmpeg. Using both ffprobe and a transcoding command on the same input stream will most likely fail unless the stream is a live stream. Only do this if you know what you're doing.

Indeed, the stream does get consumed when running ffprobe on it, that's another downside I hadn't thought of to doing it this way for file URIs.

@jwerle
Copy link
Contributor

jwerle commented Feb 13, 2020

great catch! PR is welcome ;)

@jwerle jwerle added the bug Something isn't working label Feb 13, 2020
@agrathwohl
Copy link
Contributor Author

agrathwohl commented Feb 13, 2020

Certainly @jwerle - I just have a few points I could use guidance on:

  1. Should we be creating the ReadableStream anyway to store in the returned Source object? Or was it always the intent to consume the stream created in this code?

  2. Is this an issue we ought to document for the user (that non-file:// URIs may not return all info)? And if so what's the most appropriate way to do so? GitHub wiki? README.md? :)

@agrathwohl agrathwohl self-assigned this Feb 13, 2020
@jwerle
Copy link
Contributor

jwerle commented Feb 14, 2020

I think just modifying the probe function to use this.uri (if non-null) falling backing to a stream returned from this.createReadStream(). Documenting in the README.md for now seems fine until we decide on using the Wiki

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

3 participants