Differential D8330

Open files in TagLib extractor readonly
ClosedPublic
Actions

Authored by davidk on Oct 16 2017, 11:46 AM.

Details

Reviewers
vhanda
krop
dfaure
mgallien
Group Reviewers
Frameworks
Commits
R286:098d62874591: Open files in TagLib extractor readonly
Summary

This is useful if the extractor runs sandboxed, and read-write file access is denied.
TagLib would retry it readonly, but this saves one syscall.

Test Plan
  • Autotests run fine
  • Tags are still indexed, tested with "baloosearch artist=..." and "baloosearch genre=..."

Diff Detail

Repository
R286 KFileMetaData
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidk created this revision.Oct 16 2017, 11:46 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 16 2017, 11:46 AM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
mgallien added a subscriber: mgallien.Oct 16 2017, 3:28 PM

Thanks, I tested it and it works.
I am not sure about the qWarning.
I would prefer another opinion on that.

davidk added a comment.Oct 17 2017, 7:33 AM
In D8330#156180, @mgallien wrote:

I am not sure about the qWarning.
I would prefer another opinion on that.

Some other extractors also print warnings. I think it's the only way a user can spot such problems. But let's wait for another opinion.

davidk added reviewers: vhanda, krop, dfaure.Oct 24 2017, 7:29 AM

Adding some devs who worked on kfilemetadata in the past.

mgallien accepted this revision.Nov 4 2017, 10:03 AM

If other extractors are already doing that,I guess it is good enough for now. I guess somebody may one day come up with better error logging (i.e. the user can discover it).

This revision is now accepted and ready to land.Nov 4 2017, 10:03 AM
ngraham added a comment.Nov 5 2017, 2:07 PM

Is this ready to land, or do we need some more eyeballs on it first?

ngraham added a comment.Nov 8 2017, 2:15 PM

Do you want to land this?

mgallien added a comment.Nov 8 2017, 4:16 PM

Yes exactly.

ngraham added a comment.Nov 8 2017, 4:23 PM

@davidk, I think you have commit rights; do you want to do the honors?

davidk added a comment.Nov 9 2017, 8:41 AM
In D8330#165813, @ngraham wrote:

@davidk, I think you have commit rights; do you want to do the honors?

Yes I have commit rights. I'l land it at the weekend. Thanks for your feedback!

Closed by commit R286:098d62874591: Open files in TagLib extractor readonly (authored by davidk). · Explain WhyNov 11 2017, 5:26 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/extractors/taglibextractor.cpp (24 lines)

Diff 22170

View Options

src/extractors/taglibextractor.cpp

Loading...