mediascanner2

Merge lp://staging/~jpakkane/mediascanner2/fdbackend into lp://staging/mediascanner2

  1. fdbackend
  2. Merge into trunk
Proposed by Jussi Pakkanen
Status: Needs review
Proposed branch: lp://staging/~jpakkane/mediascanner2/fdbackend
Merge into: lp://staging/mediascanner2
Diff against target: 158 lines (+56/-20)
1 file modified
src/daemon/MetadataExtractor.cc (+56/-20)
To merge this branch: bzr merge lp://staging/~jpakkane/mediascanner2/fdbackend
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Mediascanner Team Pending
Review via email: mp+250630@code.staging.launchpad.net

Commit message

Make the scanning backend use fds instead of file names.

Description of the change

Make the scanning backend use fds instead of file names. This is in preparation to creating a metadata dbus service.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:301
http://jenkins.qa.ubuntu.com/job/mediascanner2-ci/185/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-amd64-ci/22
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-armhf-ci/22
        deb: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-armhf-ci/22/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-i386-ci/22

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mediascanner2-ci/185/rebuild

review: Approve (continuous-integration)
Reply
lp://staging/~jpakkane/mediascanner2/fdbackend updated
302. By Jussi Pakkanen

Use mmap instead of reading entire file into memory.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:302
http://jenkins.qa.ubuntu.com/job/mediascanner2-ci/186/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-amd64-ci/23
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-armhf-ci/23
        deb: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-armhf-ci/23/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-i386-ci/23

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mediascanner2-ci/186/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
James Henstridge (jamesh) wrote :

Looks pretty good. A few comments left as inline comments.

Reply
lp://staging/~jpakkane/mediascanner2/fdbackend updated
303. By Jussi Pakkanen

Guard fd closing.

Revision history for this message
Jussi Pakkanen (jpakkane) wrote :

Fixed the fd closing.

However the fd uri is not as simple. Trying to use fd uris leads to test failures. I isolated the problem and filed an upstream bug: https://bugzilla.gnome.org/show_bug.cgi?id=745073

As far as the pixbuf loader thing is concerned, the api seems to be about creating a full pixbuf, meaning it will unpack the pixels and so on. We specifically use gdk_pixbuf_get_file_info because it is faster and there does not seem to be an api that takes an uri or a loader, only a file name.

Reply
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:303
http://jenkins.qa.ubuntu.com/job/mediascanner2-ci/187/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-amd64-ci/24
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-armhf-ci/24
        deb: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-armhf-ci/24/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-i386-ci/24

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mediascanner2-ci/187/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
James Henstridge (jamesh) wrote :

> Fixed the fd closing.
>
> However the fd uri is not as simple. Trying to use fd uris leads to test
> failures. I isolated the problem and filed an upstream bug:
> https://bugzilla.gnome.org/show_bug.cgi?id=745073

This looks like it is a bug in reconfiguring the pipeline. I do get a duration when I run

    gst-discoverer-1.0 fd://3 3< file1.ogg

But I get that error when I do:

    gst-discoverer-1.0 file2.ogg fd://3 3< file1.ogg

I don't get the error when using fdsrc twice though:

    gst-discoverer-1.0 fd://3 fd://4 3< file1.ogg 4< file2.ogg

>
> As far as the pixbuf loader thing is concerned, the api seems to be about
> creating a full pixbuf, meaning it will unpack the pixels and so on. We
> specifically use gdk_pixbuf_get_file_info because it is faster and there does
> not seem to be an api that takes an uri or a loader, only a file name.

Good point. It does look like a bit more is needed looking at the get_file_info implementation:

https://git.gnome.org/browse/gdk-pixbuf/tree/gdk-pixbuf/gdk-pixbuf-io.c#n1891

Anyway, the important thing is that we can use file descriptors with all the libraries we care about, so exposing this to confined processes isn't totally crazy.

Reply

Unmerged revisions

303. By Jussi Pakkanen

Guard fd closing.

302. By Jussi Pakkanen

Use mmap instead of reading entire file into memory.

301. By Jussi Pakkanen

Use fstat for the freshest, most up-to-date info.

300. By Jussi Pakkanen

Convert gst and pixbuf to use fds instead of filenames.

299. By Jussi Pakkanen

Convert exif backend to read from fd instead of filename.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches