Add preference for keeping duplicate blu ray titles #5985
Conversation
|
This patch is rather invasive due to the number of function APIs that it touches. So I would like to make a suggestion. @sr55 @galad87 what would you think of adding This would eliminate almost all the changes to libhb in this patch. And it simplifies the frontends some as well. I.e. the LinGui creates all the handles it uses on startup. The value can be changed in the handle immediately when the preference option changes. |
|
That seems like it would be much neater |
This is very strange. I'm wondering if different drives can result in reading directory entries in a different order. This is the only mechanism by which I can see the lists being different. The playlist is processed in the order that they are read from the |
|
If I'm correct about the order of reading playlists differing depending on drive, then filtering titles with duplicate clips will result in similar problems. Edit: Oh, I take that back. Titles with duplicate clips will all be filtered. There's no dependency on which it sees first because it is not comparing to another title. |
|
I'm not sure which way would be better, it's something that will be used only in hb_scan(), so having some hb_handle options and some doesn't look ideal, but then again hb_scan has got so many parameters, and the dvdnav option is already a hb_handle option. What happens if a job with one of those duplicate titles is added to the queue, then the duplicated option is disabled and the queue started? Will the queue scan find the title? |
Agreed. The 2 drives are a different filesystems (exFAT vs EXT4), which may be the key difference. Very reproducible for me though (and super annoying) |
It's used in the scan and by the reader for the job (which is part of what makes it a far-reaching change), so the I'm not sure what happens in the situation you've described on all platforms, I'll do further testing |
I wish that were the case. But when we open a particular title to encode, the index must match what was previously scanned. So the setting must be the same for both scan and encode. |
If I changed it to the approach suggested by @jstebbins, I think it wouldn't work in any UI With the patch as I currently have it: Case 1: Open title, add to queue, change setting, start queue Linux
WindowsBoth cases don't work MacBoth cases don't work I'm not immediately sure how to deal with that, I guess each GUI would need some piece of state that remembers what the setting was at the moment the title was initially scanned. A simpler solution, if you're willing to forgo configurability, would be not do anything I've done with this patch and just change |
|
I was just thinking about why the same problem doesn't exist for the minimum title length setting, but I think that's because the filtering of titles based on length is done by HandBrake rather than libbluray. Would it be possible to have libbluray always give HandBrake all the titles, then do the removal (or not) of duplicate titles within HandBrake? I think this would make the indexes stable regardless of that setting, but I'm not sure how to go about detecting duplicate titles after the call to |
|
Hmm, there is a twist when you consider allowing changing the value between scans and between scanning and encoding. Glad we are hashing this out. There's some interesting corner cases. We will need to record what value was used during scan in the Summary:
Edit: I should mention |
The information necessary is not available. The function that does this in libblueray basically parses the entire playlist structure and checks that they are completely identical in every detail. |
Shame. Oh well I'll have a look at what you've suggested in the next couple of days, thanks! |
|
@jstebbins The WinUI is pretty decoupled from libhb these days, by design. It's pretty much POCO + JSON for most of the work so it's been susceptible to scan -> encode setting changes for years. Don't really see it as much of an issue to be honest. I just throw a "Requires Restart" label after anything that it applies to. I also don't use much of the libhb preset system. Only fetch / upgrade the JSON and handle everything else natively. It's a little extra work but it's cleaner from a code perspective. If you need help with the WinUI changes, just let me know and I'm happy to do them. Just let me know what's needed. |
|
@sr55 does the Win UI represent presets or jobs with significantly different json than libhb? Because hb_preset_job_init_json() is pretty straight forward. Just seems like it would save you quite a lot of synchronization work. |
|
Presets is standard JSON but is converted to an internal representation which isn't the same. I've never used the job model. The UI has it's own modelling which again gets converted to the JSON. It's not a lot of code to convert back/forth really so was never worth changing and it allows me to extend the model for some UI specific stuff. It would be major work to try change it at this point. |
|
I got the Linux GUI working with the suggested changes, you can change that setting all you like after the initial scan, and it still works just fine. You can even add to the queue, change the setting, then restart the application and the queued item still does what it should. I'm struggling with the Mac GUI though, I was expecting it to work similarly to the linux ui. Here's my WIP, on top of 7be4117, just removing the changes I made to make the GUI override diff --git a/macosx/HBJob+HBJobConversion.m b/macosx/HBJob+HBJobConversion.m
index d6ee08699..8d6d52718 100644
--- a/macosx/HBJob+HBJobConversion.m
+++ b/macosx/HBJob+HBJobConversion.m
@@ -58,8 +58,6 @@ - (hb_job_t *)hb_job
{
job->hw_decode = HB_DECODE_SUPPORT_VIDEOTOOLBOX | HB_DECODE_SUPPORT_FORCE_HW;
}
-
- job->keep_duplicate_titles = self.keepDuplicateTitles;
// Title Angle for dvdnav
job->angle = self.angle;
diff --git a/macosx/HBJob.h b/macosx/HBJob.h
index 03bb5c69f..e44c65525 100644
--- a/macosx/HBJob.h
+++ b/macosx/HBJob.h
@@ -80,7 +80,6 @@ typedef NS_ENUM(NSUInteger, HBJobHardwareDecoderUsage) {
@property (nonatomic, readwrite) BOOL metadataPassthru;
@property (nonatomic, readwrite) HBJobHardwareDecoderUsage hwDecodeUsage;
-@property (nonatomic, readwrite) BOOL keepDuplicateTitles;
@property (nonatomic, readwrite, weak, nullable) NSUndoManager *undo;
diff --git a/macosx/HBJob.m b/macosx/HBJob.m
index 35cb99205..39ea40f3e 100644
--- a/macosx/HBJob.m
+++ b/macosx/HBJob.m
@@ -425,7 +425,6 @@ - (instancetype)copyWithZone:(NSZone *)zone
copy->_metadataPassthru = _metadataPassthru;
copy->_hwDecodeUsage = _hwDecodeUsage;
- copy->_keepDuplicateTitles = _keepDuplicateTitles;
}
return copy;
@@ -491,7 +490,6 @@ - (void)encodeWithCoder:(NSCoder *)coder
encodeBool(_metadataPassthru);
encodeInteger(_hwDecodeUsage);
- encodeBool(_keepDuplicateTitles);
}
- (instancetype)initWithCoder:(NSCoder *)decoder
@@ -537,7 +535,6 @@ - (instancetype)initWithCoder:(NSCoder *)decoder
decodeBool(_metadataPassthru);
decodeInteger(_hwDecodeUsage); if (_hwDecodeUsage != HBJobHardwareDecoderUsageNone && _hwDecodeUsage != HBJobHardwareDecoderUsageAlways && _hwDecodeUsage != HBJobHardwareDecoderUsageFullPathOnly) { goto fail; }
- decodeBool(_keepDuplicateTitles);
return self;
}
diff --git a/macosx/HBPreviewGenerator.m b/macosx/HBPreviewGenerator.m
index a161e908f..5d8bbd8b7 100644
--- a/macosx/HBPreviewGenerator.m
+++ b/macosx/HBPreviewGenerator.m
@@ -273,8 +273,6 @@ - (BOOL) createMovieAsyncWithImageAtIndex: (NSUInteger) index duration: (NSUInte
{
job.hwDecodeUsage = HBJobHardwareDecoderUsageNone;
}
-
- job.keepDuplicateTitles = [NSUserDefaults.standardUserDefaults boolForKey:HBKeepDuplicateTitles];
// Init the libhb core
NSInteger level = [NSUserDefaults.standardUserDefaults integerForKey:HBLoggingLevel];
diff --git a/macosx/HBQueueWorker.m b/macosx/HBQueueWorker.m
index e557ced99..fc9cb7364 100644
--- a/macosx/HBQueueWorker.m
+++ b/macosx/HBQueueWorker.m
@@ -224,8 +224,6 @@ - (void)realEncodeItem:(HBQueueJobItem *)item
{
job.hwDecodeUsage = HBJobHardwareDecoderUsageNone;
}
-
- job.keepDuplicateTitles = [NSUserDefaults.standardUserDefaults boolForKey:HBKeepDuplicateTitles];
// Progress handler
void (^progressHandler)(HBState state, HBProgress progress, NSString *info) = ^(HBState state, HBProgress progress, NSString *info) |
|
The Mac UI doesn't use the json API, so it's a bit different. However you were missing only one thing: reading the title keep_duplicate_titles and setting it in the job: diff --git a/macosx/HBJob+HBJobConversion.m b/macosx/HBJob+HBJobConversion.m
index d6ee08699..8d6d52718 100644
--- a/macosx/HBJob+HBJobConversion.m
+++ b/macosx/HBJob+HBJobConversion.m
@@ -58,8 +58,6 @@ - (hb_job_t *)hb_job
{
job->hw_decode = HB_DECODE_SUPPORT_VIDEOTOOLBOX | HB_DECODE_SUPPORT_FORCE_HW;
}
-
- job->keep_duplicate_titles = self.keepDuplicateTitles;
// Title Angle for dvdnav
job->angle = self.angle;
diff --git a/macosx/HBJob.m b/macosx/HBJob.m
index 35cb99205..9707f91df 100644
--- a/macosx/HBJob.m
+++ b/macosx/HBJob.m
@@ -52,6 +52,8 @@ - (nullable instancetype)initWithTitle:(HBTitle *)title preset:(HBPreset *)prese
NSParameterAssert(title);
NSParameterAssert(preset);
+ _keepDuplicateTitles = title.keepDuplicateTitles;
+
_title = title;
_titleIdx = title.index;
_stream = title.isStream;
diff --git a/macosx/HBPreviewGenerator.m b/macosx/HBPreviewGenerator.m
index a161e908f..5d8bbd8b7 100644
--- a/macosx/HBPreviewGenerator.m
+++ b/macosx/HBPreviewGenerator.m
@@ -273,8 +273,6 @@ - (BOOL) createMovieAsyncWithImageAtIndex: (NSUInteger) index duration: (NSUInte
{
job.hwDecodeUsage = HBJobHardwareDecoderUsageNone;
}
-
- job.keepDuplicateTitles = [NSUserDefaults.standardUserDefaults boolForKey:HBKeepDuplicateTitles];
// Init the libhb core
NSInteger level = [NSUserDefaults.standardUserDefaults integerForKey:HBLoggingLevel];
diff --git a/macosx/HBQueueWorker.m b/macosx/HBQueueWorker.m
index e557ced99..68c37d0d9 100644
--- a/macosx/HBQueueWorker.m
+++ b/macosx/HBQueueWorker.m
@@ -199,7 +199,7 @@ - (void)encodeItem:(HBQueueJobItem *)item
minDuration:0
keepPreviews:NO
hardwareDecoder:[NSUserDefaults.standardUserDefaults boolForKey:HBUseHardwareDecoder]
- keepDuplicateTitles:[NSUserDefaults.standardUserDefaults boolForKey:HBKeepDuplicateTitles]
+ keepDuplicateTitles:item.job.keepDuplicateTitles
progressHandler:progressHandler
completionHandler:completionHandler];
}
@@ -224,8 +224,6 @@ - (void)realEncodeItem:(HBQueueJobItem *)item
{
job.hwDecodeUsage = HBJobHardwareDecoderUsageNone;
}
-
- job.keepDuplicateTitles = [NSUserDefaults.standardUserDefaults boolForKey:HBKeepDuplicateTitles];
// Progress handler
void (^progressHandler)(HBState state, HBProgress progress, NSString *info) = ^(HBState state, HBProgress progress, NSString *info)
diff --git a/macosx/HBTitle.h b/macosx/HBTitle.h
index 46c71f71f..3227b43dc 100644
--- a/macosx/HBTitle.h
+++ b/macosx/HBTitle.h
@@ -70,6 +70,8 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic, readonly) NSURL *url;
+@property (nonatomic, readonly) BOOL keepDuplicateTitles;
+
@property (nonatomic, readonly) int index;
@property (nonatomic, readonly) int angles;
@property (nonatomic, readonly) int duration;
diff --git a/macosx/HBTitle.m b/macosx/HBTitle.m
index c41a8de54..daa4262b5 100644
--- a/macosx/HBTitle.m
+++ b/macosx/HBTitle.m
@@ -424,6 +424,11 @@ - (NSURL *)url
return [NSURL fileURLWithPath:@(_hb_title->path)];
}
+- (BOOL)keepDuplicateTitles
+{
+ return self.hb_title->keep_duplicate_titles;
+}
+
- (int)index
{
return self.hb_title->index;
|
|
@galad87 mac UI appears to call hb_preset_job_init() which would copy |
|
@samhutchins Linux and libhb changes look good to me on an initial reading. I'll read everything through a second time once the rest is working 😄 |
|
Only parts of the result of hb_preset_job_init() are copied back to the objective-c HBJob instance. Yes we could copy keep_duplicate_titles from that too, it would save only 4 or 5 lines of code. |
|
When reloading a job from the queue it needs to use the keepDuplicateTitles value from the job, instead of the one from the preferences, here's the updated changes for the Mac GUI: diff --git a/macosx/HBController.m b/macosx/HBController.m
index f92a96161..2211f6dfa 100644
--- a/macosx/HBController.m
+++ b/macosx/HBController.m
@@ -648,7 +648,7 @@ - (NSModalResponse)runCopyProtectionAlert
/**
* Here we actually tell hb_scan to perform the source scan, using the path to source and title number
*/
-- (void)scanURLs:(NSArray<NSURL *> *)fileURLs titleIndex:(NSUInteger)index completionHandler:(void(^)(NSArray<HBTitle *> *titles))completionHandler
+- (void)scanURLs:(NSArray<NSURL *> *)fileURLs titleIndex:(NSUInteger)index keepDuplicateTitles:(BOOL)keepDuplicateTitles completionHandler:(void(^)(NSArray<HBTitle *> *titles))completionHandler
{
[self showWindow:self];
@@ -682,13 +682,14 @@ - (void)scanURLs:(NSArray<NSURL *> *)fileURLs titleIndex:(NSUInteger)index compl
{
NSUInteger hb_num_previews = [NSUserDefaults.standardUserDefaults integerForKey:HBPreviewsNumber];
NSUInteger min_title_duration_seconds = [NSUserDefaults.standardUserDefaults integerForKey:HBMinTitleScanSeconds];
+ BOOL hardwareDecoder = [NSUserDefaults.standardUserDefaults boolForKey:HBUseHardwareDecoder];
[self.core scanURLs:fileURLs
titleIndex:index
previews:hb_num_previews minDuration:min_title_duration_seconds
keepPreviews:YES
- hardwareDecoder:[NSUserDefaults.standardUserDefaults boolForKey:HBUseHardwareDecoder]
- keepDuplicateTitles:[NSUserDefaults.standardUserDefaults boolForKey:HBKeepDuplicateTitles]
+ hardwareDecoder:hardwareDecoder
+ keepDuplicateTitles:keepDuplicateTitles
progressHandler:^(HBState state, HBProgress progress, NSString *info)
{
self.sourceLabel.stringValue = info;
@@ -810,7 +811,7 @@ - (void)openURLs:(NSArray<NSURL *> *)fileURLs recursive:(BOOL)recursive titleInd
NSArray<NSURL *> *subtitlesFileURLs = [HBUtilities extractURLs:expandedFileURLs withExtension:HBUtilities.supportedExtensions];
NSArray<NSURL *> *trimmedFileURLs = [HBUtilities trimURLs:expandedFileURLs withExtension:excludedExtensions];
- [self scanURLs:trimmedFileURLs titleIndex:index completionHandler:^(NSArray<HBTitle *> *titles)
+ [self scanURLs:trimmedFileURLs titleIndex:index keepDuplicateTitles:[NSUserDefaults.standardUserDefaults boolForKey:HBKeepDuplicateTitles] completionHandler:^(NSArray<HBTitle *> *titles)
{
NSArray<NSURL *> *baseURLs = [HBUtilities baseURLs:trimmedFileURLs];
@@ -872,7 +873,7 @@ - (void)openJob:(HBJob *)job completionHandler:(void (^)(BOOL result))handler
[job refreshSecurityScopedResources];
self.fileTokens = @[[HBSecurityAccessToken tokenWithObject:job.fileURL]];
- [self scanURLs:@[job.fileURL] titleIndex:job.titleIdx completionHandler:^(NSArray<HBTitle *> *titles)
+ [self scanURLs:@[job.fileURL] titleIndex:job.titleIdx keepDuplicateTitles:job.keepDuplicateTitles completionHandler:^(NSArray<HBTitle *> *titles)
{
if (titles.count)
{
diff --git a/macosx/HBJob+HBJobConversion.m b/macosx/HBJob+HBJobConversion.m
index d6ee08699..8d6d52718 100644
--- a/macosx/HBJob+HBJobConversion.m
+++ b/macosx/HBJob+HBJobConversion.m
@@ -58,8 +58,6 @@ - (hb_job_t *)hb_job
{
job->hw_decode = HB_DECODE_SUPPORT_VIDEOTOOLBOX | HB_DECODE_SUPPORT_FORCE_HW;
}
-
- job->keep_duplicate_titles = self.keepDuplicateTitles;
// Title Angle for dvdnav
job->angle = self.angle;
diff --git a/macosx/HBJob.m b/macosx/HBJob.m
index 35cb99205..9707f91df 100644
--- a/macosx/HBJob.m
+++ b/macosx/HBJob.m
@@ -52,6 +52,8 @@ - (nullable instancetype)initWithTitle:(HBTitle *)title preset:(HBPreset *)prese
NSParameterAssert(title);
NSParameterAssert(preset);
+ _keepDuplicateTitles = title.keepDuplicateTitles;
+
_title = title;
_titleIdx = title.index;
_stream = title.isStream;
diff --git a/macosx/HBPreviewGenerator.m b/macosx/HBPreviewGenerator.m
index a161e908f..5d8bbd8b7 100644
--- a/macosx/HBPreviewGenerator.m
+++ b/macosx/HBPreviewGenerator.m
@@ -273,8 +273,6 @@ - (BOOL) createMovieAsyncWithImageAtIndex: (NSUInteger) index duration: (NSUInte
{
job.hwDecodeUsage = HBJobHardwareDecoderUsageNone;
}
-
- job.keepDuplicateTitles = [NSUserDefaults.standardUserDefaults boolForKey:HBKeepDuplicateTitles];
// Init the libhb core
NSInteger level = [NSUserDefaults.standardUserDefaults integerForKey:HBLoggingLevel];
diff --git a/macosx/HBQueueWorker.m b/macosx/HBQueueWorker.m
index e557ced99..68c37d0d9 100644
--- a/macosx/HBQueueWorker.m
+++ b/macosx/HBQueueWorker.m
@@ -199,7 +199,7 @@ - (void)encodeItem:(HBQueueJobItem *)item
minDuration:0
keepPreviews:NO
hardwareDecoder:[NSUserDefaults.standardUserDefaults boolForKey:HBUseHardwareDecoder]
- keepDuplicateTitles:[NSUserDefaults.standardUserDefaults boolForKey:HBKeepDuplicateTitles]
+ keepDuplicateTitles:item.job.keepDuplicateTitles
progressHandler:progressHandler
completionHandler:completionHandler];
}
@@ -224,8 +224,6 @@ - (void)realEncodeItem:(HBQueueJobItem *)item
{
job.hwDecodeUsage = HBJobHardwareDecoderUsageNone;
}
-
- job.keepDuplicateTitles = [NSUserDefaults.standardUserDefaults boolForKey:HBKeepDuplicateTitles];
// Progress handler
void (^progressHandler)(HBState state, HBProgress progress, NSString *info) = ^(HBState state, HBProgress progress, NSString *info)
diff --git a/macosx/HBTitle.h b/macosx/HBTitle.h
index 46c71f71f..3227b43dc 100644
--- a/macosx/HBTitle.h
+++ b/macosx/HBTitle.h
@@ -70,6 +70,8 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic, readonly) NSURL *url;
+@property (nonatomic, readonly) BOOL keepDuplicateTitles;
+
@property (nonatomic, readonly) int index;
@property (nonatomic, readonly) int angles;
@property (nonatomic, readonly) int duration;
diff --git a/macosx/HBTitle.m b/macosx/HBTitle.m
index c41a8de54..daa4262b5 100644
--- a/macosx/HBTitle.m
+++ b/macosx/HBTitle.m
@@ -424,6 +424,11 @@ - (NSURL *)url
return [NSURL fileURLWithPath:@(_hb_title->path)];
}
+- (BOOL)keepDuplicateTitles
+{
+ return self.hb_title->keep_duplicate_titles;
+}
+
- (int)index
{
return self.hb_title->index; |
samhutchins
force-pushed
the
keep_duplicate_titles
branch
from
b8351a7
to
a7b8037
Compare
|
@galad87 Thanks! I think I've got everything now @sr55 I think I've got the windows UI working, but I feel pretty shaky about it. @jstebbins Thanks! Once everyone's happy I'll probably force-push a squashed commit if that's OK |
samhutchins
force-pushed
the
keep_duplicate_titles
branch
from
60fec9d
to
2ab11b0
Compare
|
I'll do a review / test on the Windows side when I get a moment. Cheers |
Description of Change:
Currently HandBrake filters out duplicate titles when opening a Blu Ray, which has a couple of issues for more advanced usage:
I've had a crack at implementing this for all the user interfaces, but I'm far from an expert in any of them. I'm keen to learn though :-)
CLI:
--keep-duplicate-titlesMac: Settings -> Advanced
Linux: Preferences -> General
Windows: Preferences -> Advanced
I've tried to put the new setting near the "min duration" config in all cases.
I've also deleted a method in the Linux GUI in
hb-backend.c:ghb_backend_queue_scan. I couldn't find anywhere this method was actually called, so I just removed itTested on: