Opened 14 years ago

Closed 14 years ago

#603 closed enhancement (fixed)

Removable media support in MythGallery

Reported by: Aaron McCarthy <mccarthy.aaron@…> Owned by: danielk
Priority: minor Milestone: 0.20
Component: mythgallery Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

This patch adds removable media support to MythGallery. It includes basic file manipulations (copy, move, delete, create dir). Due to the large number of added actions I've added a submenu. This requires changes to the theme xml which I have done for blue and default.

The new png image should be copied to mythplugins/mythgallery/mythgallery/images.

You can mark files and directories, with the MARK ('T') key, and operate on all the marked files in one operation. Marking is persistent across directory changes.

The thumbcache is always stored in the users home directory for removable media.

This patch depends on #601

Attachments (8)

gallery_removable.diff (31.1 KB) - added by Aaron McCarthy <mccarthy.aaron@…> 14 years ago.
gallery_blue_theme.diff (1.0 KB) - added by Aaron McCarthy <mccarthy.aaron@…> 14 years ago.
Update blue theme to support submenu
gallery-mark.png (1.2 KB) - added by Aaron McCarthy <mccarthy.aaron@…> 14 years ago.
new binary file mythplugins/mythgallery/mythgallery/images/gallery-mark.png
gallery_removable_v2.diff (31.4 KB) - added by Aaron McCarthy <mccarthy.aaron@…> 14 years ago.
Updated to apply cleanly to svn
gallery_removable_v3.patch (37.0 KB) - added by danielk 14 years ago.
updated version
gallery_removable_v4.diff (36.7 KB) - added by Aaron McCarthy <mccarthy.aaron@…> 14 years ago.
mythmedia_locking.diff (8.0 KB) - added by Aaron McCarthy <mccarthy.aaron@…> 14 years ago.
gallery_removable_v5.diff (39.7 KB) - added by Aaron McCarthy <mccarthy.aaron@…> 14 years ago.

Download all attachments as: .zip

Change History (14)

Changed 14 years ago by Aaron McCarthy <mccarthy.aaron@…>

Attachment: gallery_removable.diff added

Changed 14 years ago by Aaron McCarthy <mccarthy.aaron@…>

Attachment: gallery_blue_theme.diff added

Update blue theme to support submenu

Changed 14 years ago by Aaron McCarthy <mccarthy.aaron@…>

Attachment: gallery-mark.png added

new binary file mythplugins/mythgallery/mythgallery/images/gallery-mark.png

comment:1 Changed 14 years ago by danielk

Milestone: 0.20
Owner: changed from Isaac Richards to danielk
Status: newassigned

Changed 14 years ago by Aaron McCarthy <mccarthy.aaron@…>

Attachment: gallery_removable_v2.diff added

Updated to apply cleanly to svn

Changed 14 years ago by danielk

Attachment: gallery_removable_v3.patch added

updated version

comment:2 Changed 14 years ago by danielk

Aaron can you look at this again?

I didn't get as far as testing importing code, but I looked at the tag/copy/move/delete UI.

There are some UI improvements which would make this much easier to use:

  • Clear marks after operation
  • Allow wholesale clearing all marks
  • If there are no marks and operation is delete, mark then delete selected item
  • Exit submenu on ESCAPE
  • Exit "Show Devices" on exit from submenu
  • Bind DELETE key to 'Delete Marked'
  • Show progress bar while performing operation. (The first thing I tried to copy took several minutes...)

Plus there some code problems that need to be addressed:

  • Use Qt or C API for renaming/unlinking/copying files or directories. (Using the shell means needing to worry about shell escaping... As is, the core is unsafe.)
  • GetMedias?() is unsafe if a device is removed/unmounted while you are looking at its returned list...
  • Avoid using #define's and #ifdef's when possible. For example you disable some code when _WIN32 is defined in IconView? where it doesn't look like you need to.

Changed 14 years ago by Aaron McCarthy <mccarthy.aaron@…>

Attachment: gallery_removable_v4.diff added

comment:3 Changed 14 years ago by Aaron McCarthy <mccarthy.aaron@…>

Daniel,

I've just attached a new version of the patch that addresses most of the issues you mentioned. The only ones I haven't fixed are

  • Exit "Show Devices" on exit from submenu

Pressing escape while in Show Devices (and the menu is not active) will return you to the root gallery directory.

  • GetMedias?() is unsafe if a device is removed/unmounted while you are

looking at its returned list...

I will need to implement some sort of locking for this.

There are some UI improvements which would make this much easier to use:

  • Clear marks after operation

I implemented it like this because I have another patch which allows adding keywords to the selected set of images. My workflow is select images on media, copy/move to gallery, add keywords, clear selection.

For now I will clear the marks after the operation.

Marks are cleared after copy, move.

  • Allow wholesale clearing all marks

What do you mean by this? Under the Marking Submenu there is a "Clear Marked" item that clears all the currently marked items.

  • If there are no marks and operation is delete, mark then delete

selected item

  • Bind DELETE key to 'Delete Marked'

Done. I have also updated the singleview classes to use the new delete functions in GalleryUtil?. Fixed a bug when in singleview mode and images were delete, after exiting back to gallery view the directory listing was not updated.

  • Exit submenu on ESCAPE

Done.

Plus there some code problems that need to be addressed:

  • Use Qt or C API for renaming/unlinking/copying files or directories.

(Using the shell means needing to worry about shell escaping... As is, the core is unsafe.)

Done for Deleting.

  • Avoid using #define's and #ifdef's when possible. For example you

disable some code when _WIN32 is defined in IconView? where it doesn't look like you need to.

I have removed some of the #ifdef's which shouldn't cause problems on win32 (I have no way of testing this). But because the code uses MediaMonitor?, which is only compiled on unix system I have left a few in.

  • Show progress bar while performing operation. (The first thing I tried

to copy took several minutes...)

Done, but it is not overly smart. Nested copies, such as when a directory is marked, are only reported as 1 item being copied/moved.

Changed 14 years ago by Aaron McCarthy <mccarthy.aaron@…>

Attachment: mythmedia_locking.diff added

Changed 14 years ago by Aaron McCarthy <mccarthy.aaron@…>

Attachment: gallery_removable_v5.diff added

comment:4 Changed 14 years ago by Aaron McCarthy <mccarthy.aaron@…>

I have just uploaded two new files which I believe addresses most of the issues.

Could you clarify these three remaining points Allow wholesale clearing of marks As per my last comment to this ticket there is a menu option that I believe does this.

Exit "Show Devices" on exit from submenu I'm not sure this is needed, and may cause problems. If you exit from "Show Devices" when you exit the menu how do you browse into the device?

Avoid using #define and #ifdef when possible. I believe I have minimised the use of ifdefs

The final patch now consists of 4 parts gallery_blue_theme.diff gallery-mark.png mythmedia_locking.diff gallery_removable_v5.diff

Improvements I have made in this version: I've added locking functions to MediaMonitor?. Other threads can now pass around and store MythMediaDevice? pointers without locking. Before dereferencing the pointer, calling member functions etc the pointer must be validated and locked, when finished it should be unlocked. I have fixed up all places that use MythMediaDevices? in mythtv and mythplugins to use this scheme.

I've implemented GalleryUtil::FileCopy? and GalleryUtil::FileMove? using library functions instead of calling external commands.

comment:5 Changed 14 years ago by danielk

(In [9455]) References #603. Adds some basic locking to media monitor.

This allows you to eject a device manually, or otherwise do so outside MythTV, without potentially causing a segfault.

comment:6 Changed 14 years ago by danielk

Resolution: fixed
Status: assignedclosed

(In [9458]) Closes #603. Adds tag, copy, move and import functionality.

You need to install pmount and to add the udev stuff from contrib for easy importing, but you can still import this without that if you have some sort of automounting set up.

Note: See TracTickets for help on using tickets.