1.14.0 ACTION_EJECTCD doesn't work (1 Viewer)

CyberSimian

Test Group
  • Team MediaPortal
  • June 10, 2013
    2,873
    1,801
    Southampton
    Home Country
    United Kingdom United Kingdom
    I'll see if I can build a 1.12 version for your production system, and maybe throw in a 1.15 PR version as optional.
    Thank you for this. :) Having multiple MP versions in common use must be a maintenance nightmare. :(
    I have downloaded the 1.12 and 1.15pre versions and will test them later today or tomorrow.

    Please be aware that it's difficult if not impossible to detect the difference between empty drive and open tray. Thus writing logic to always open and close the tray appropriately with only one button is tricky
    One feels that the EJECT button on the remote ought really to be TOGGLE_CD_DRAWER. But if Windows does not allow "drawer open" to be distinguished from "empty drive", one is stuck. :( (I suppose that state tracking of the CD drive might provide a solution, but that is probably a lot more complicated, and possibly not "watertight".)

    due to the difficulty in distinguishing closed empty tray from open tray, I think we should modify this so that eject only ejects and load only loads. It would be nice to be smart, but functional is the ultimate priority.
    Yes, I agree with this; it would provide more consistent operation.

    Using either the load or eject ( :rolleyes: ) menu items to close the tray containing an audio CD does not give me duplicates.
    The duplicate entries did not last very long; after several further button-presses on the remote, the duplicates disappeared (probably when I caused MP to redraw the screen completely). Hopefully matching the fix version to the MP version will avoid this happening.

    Thank you for taking the time to address this issue. :)

    -- from CyberSimian in the UK
     

    CyberSimian

    Test Group
  • Team MediaPortal
  • June 10, 2013
    2,873
    1,801
    Southampton
    Home Country
    United Kingdom United Kingdom
    All that remains to be mentioned are the attached patches for 1.12, 1.14 and 1.15 PR. Please test as you're able, and be sure to cover the videos, music and pictures sections.
    I have now tested the fixes for MP 1.12. I tested all three panels ("Music", "Pictures", and "Videos"), and nothing looked incorrect -- my video folders were present on the "Videos" panel, and the duplication of the list on the "Music" panel did not occur.

    The behaviour of the EJECT button on the remote, and of the "Load" and "Eject" entries on the context menus, were as expected -- they performed their named functions, and did not act as toggles. (y)

    However, there was one aspect of the dialogue that stuck me as less than optimal. I don't know where this arises -- it might be the skin, or it might be hardcoded behaviour somewhere else within MP. This is the scenario:

    (1) I have arranged my video folders so that the entry for the CD/DVD drive is not first in the list.

    (2) I move the focus to the entry for the CD/DVD drive, display the context menu, and select "Eject" to open the CD drawer: (Edit: this image incorrectly shows "Load" with the focus -- it should be "Eject")

    13-24-17.jpg

    (3) The drawer opens, and the focus moves to the first entry in the list :(. I load the DVD, move the focus back to the entry for the CD/DVD drive, display the context menu, and select "Load". The drive spins up, and the "DVD open" pop-up appears:

    13-24-52.jpg

    (4) If I select "No" at this point, the focus once again moves to the first entry in the list :(:

    13-25-03.jpg

    I think that the focus ought to remain on the entry for the CD/DVD drive in both cases (the "principle of least surprise"). The skin is the discontinued "DefaultWide", so if this behaviour originates with the skin, it is unlikely to be changed. I will try MP 1.15pre tomorrow and see if the current "DefaultWideHD" behaves in the same way (but I think that this is a separate issue, so not part of the fix for the eject button).

    -- from CyberSimian in the UK
     

    mm1352000

    Retired Team Member
  • Premium Supporter
  • September 1, 2008
    21,577
    8,224
    Home Country
    New Zealand New Zealand
    One feels that the EJECT button on the remote ought really to be TOGGLE_CD_DRAWER. But if Windows does not allow "drawer open" to be distinguished from "empty drive", one is stuck. :( (I suppose that state tracking of the CD drive might provide a solution, but that is probably a lot more complicated, and possibly not "watertight".)
    I think we're on the same page. ;)

    I agree with your sentiment, and had thought about the possibility of state tracking. Issues with that approach:
    1. Not all drives have a tray.
    2. Some trays can't be closed by command from the PC (eg. many modern laptop trays - they can be ejected by the PC, but the person has to physically close the tray because it's not driven by a motor).
    3. State tracking is hard if a person doesn't consistently use the MP commands to load/eject discs.
    It just seems like a whole can of worms that I really don't want to dive into. There's every chance of making the behaviour/situation worse.

    The duplicate entries did not last very long...
    ...and the duplication of the list on the "Music" panel did not occur.
    Great! I really didn't want to have to step through that code to figure out what was going on. ;)

    However, there was one aspect of the dialogue that stuck me as less than optimal.
    Once again, I generally agree with your perspective.

    From the code, it looks like MP does try to do what you're suggesting. However it seems that function is based on the item name. Because disc insertion/ejection changes the name, the "tracking" fails. This would be independent of the skin.
     

    CyberSimian

    Test Group
  • Team MediaPortal
  • June 10, 2013
    2,873
    1,801
    Southampton
    Home Country
    United Kingdom United Kingdom
    All that remains to be mentioned are the attached patches for 1.12, 1.14 and 1.15 PR. Please test as you're able
    I have not tested the 1.15pre fix yet, as I need to boot my test partition to do that. However, whilst my production partition was running, I thought that I would see what happens when there is more than one CD/DVD drive in the system. So I plugged in my USB CD/DVD drive and loaded a DVD into each drive (using the physical buttons on the drives to open and close the drawers).

    The context menu worked fine -- moving the focus to a CD/DVD drive enabled the "Eject" action on the menu, and selecting that action caused the correct drawer to be opened. :)

    Similarly, with both drawers open, selecting the "Load" action caused the correct drawer to be closed. :)

    But now we come to the EJECT button on the remote control. If neither CD/DVD drive had the focus, pressing the EJECT button opened the drawer for the first CD/DVD drive in the list of folders. In my case this happened to be the USB drive, accessed as "F:" (the built-in drive is "O:"). This behaviour seems OK. :)

    The unexpected behaviour is what happens when one of the CD/DVD drives has the focus. In this case I would expect EJECT to open the drawer for the drive with the focus, but it does not; it always acts on the first CD/DVD drive in the list. :(

    This is possibly a nuance in the dialogue, which may not have been designed to work that way. I think that it should work that way, but this need not hold up including the fixes in the forthcoming release. (y)

    -- from CyberSimian in the UK
     

    HTPCSourcer

    Retired Team Member
  • Premium Supporter
  • May 16, 2008
    11,418
    2,335
    Home Country
    Germany Germany
    Issues with that approach:
    1. Not all drives have a tray.
    2. Some trays can't be closed by command from the PC (eg. many modern laptop trays - they can be ejected by the PC, but the person has to physically close the tray because it's not driven by a motor).
    3. State tracking is hard if a person doesn't consistently use the MP commands to load/eject discs.
    As an additional comment I would add that the issue is likely to resolve itself in terms of drives becoming more and more obsolete. While 2. essentially applies to notebooks, it is becoming quite uncommon for manufacturers to provide drives for their hardware.

    By no means does this imply that I would not take such requests seriously. However, we may want to consider the shrinking importance of such hardware. In my own case all clients don't have a drive anymore - everything is stored on the central repository.
     

    CyberSimian

    Test Group
  • Team MediaPortal
  • June 10, 2013
    2,873
    1,801
    Southampton
    Home Country
    United Kingdom United Kingdom
    However, we may want to consider the shrinking importance of such hardware. In my own case all clients don't have a drive anymore - everything is stored on the central repository.
    Speaking personally, I wouldn't want to spend the time to capture all of the DVDs in my collection in order to store them on my NAS. Each DVD is watched sufficiently infrequently that having physically to load it in order to watch it is not an onerous task.

    -- from CyberSimian in the UK
     

    HTPCSourcer

    Retired Team Member
  • Premium Supporter
  • May 16, 2008
    11,418
    2,335
    Home Country
    Germany Germany
    Everybody as he likes. Nonetheless as long as MP is not able to take the DVD out of its case, load it into the drive and then put it back into the case, we are talking about tertiary things IMHO. All that the Eject button does is to replace pushing the open/close button of the drive, and your hand will always be on it in order to insert and remove the DVD.

    No offense meant, but this starts to sound like heeding multiple requests of a single client.
     

    mm1352000

    Retired Team Member
  • Premium Supporter
  • September 1, 2008
    21,577
    8,224
    Home Country
    New Zealand New Zealand
    this bug need Jira ticket ?
    Yes. I haven't created one yet.

    No, I think completely unrelated.

    I have not tested the 1.15pre fix yet, as I need to boot my test partition to do that.
    Okay. I think that's all we're really waiting on at this point. Note that I will be AFK for approximately a week starting Sunday (NZ time). It'd be good to knock this issue out before then if possible.

    The context menu worked fine -- moving the focus to a CD/DVD drive enabled the "Eject" action on the menu, and selecting that action caused the correct drawer to be opened. :)

    Similarly, with both drawers open, selecting the "Load" action caused the correct drawer to be closed. :)
    Correct - that's expected. The context menu entries specify the drive they operate on.

    But now we come to the EJECT button on the remote control...
    What you've observed is what I'd expect given the code that is present.

    The eject button on the remote has no relation/connection to the focussed item. They're handled by two different sections of code which for all intents and purposes are unconnected. Eject on the remote literally can only eject the default disc drive. "Default" is defined by Windows. Presumably it's alphabetical on the driver letter. It has absolutely no relation to the focus position or folder order in MP's video, music or pictures plugins.

    This is possibly a nuance in the dialogue, which may not have been designed to work that way. I think that it should work that way...
    I haven't got a lot of time to dive into the code this morning, and I probably shouldn't include functionality changes in a bug fix either. However you could try the attached patch. It's for 1.15 PR only. Sorry, no time to do other versions... and this will give you a little incentive to boot up that test partition. ;)

    The nature of the change to enable what you requested is that the "base MP event listener" must be told to ignore eject actions and let videos/music/pictures handle the action in certain scenarios. It's possible that my change doesn't cover all the scenarios you want, but this is a start.
     

    Attachments

    • 1.15 PR eject fix + enhance v3.zip
      1.7 MB

    Users who are viewing this thread

    Top Bottom