[Poll] Expose present surface object as in Framegrabber as event? (1 Viewer)

Add the patch to SVN?


  • Total voters
    7

gemx

Retired Team Member
  • Premium Supporter
  • October 31, 2006
    1,972
    539
    Home Country
    Germany Germany
    Hi,

    while working on the Atmolight plugin (see here https://forum.team-mediaportal.com/...in-update-13-01-2010-v1-1-a-75840/#post557386) i noticed that the current method in the framebuffer always copies the whole image from the GPU which is a big CPU eater.
    Even more when you think about 1080i/p video content.
    In most cases it is enough to get only a small portion of the screen (e.g. for black bar detection) or get a smaller, resized image like e.g. in my case.

    I tried many different ways to capture the current screen content but the framegrabber was the only solution since it gets feeded directly by PlaneSurface.PresentImage(...).

    To give Plugin developers the freedom of choice how they would like the current image i made the attached small patch.
    It just exposes an OnNewFrameEvent from the framegrabber, where the current image is handed over before it's processed by the framegrabber itself.

    It's just a very small patch and until this event is not hooked to any code it won't be used anyway but maybe of great help to take CPU load from the framegrabber.

    Ok, I know, we have a feature freeze but this is not really a feature - it helps to reduce CPU load in MP if used by a plugin and if not it doesn't have any effect or side effect at all.

    This is a bit like begging - but please vote for "yes" because otherwise my (and probably that of many other) Atmolight Hardware will never run smooth with MP :p

    So, what do you think?

    Please vote :)

    :D

    The patch is here
    Code:
    Index: Core/Player/FrameGrabber.cs
    ===================================================================
    --- Core/Player/FrameGrabber.cs    (revision 24717)
    +++ Core/Player/FrameGrabber.cs    (working copy)
    @@ -45,6 +45,9 @@
     
         private FrameGrabber() {}
     
    +    public delegate void NewFrameHandler(Int16 width, Int16 height, Int16 arWidth, Int16 arHeight, uint pSurface);
    +    public event NewFrameHandler OnNewFrame;
    +
         public static FrameGrabber GetInstance()
         {
           if (instance == null)
    @@ -120,6 +123,16 @@
         /// <param name="pSurface"></param>
         public void OnFrame(Int16 width, Int16 height, Int16 arWidth, Int16 arHeight, uint pSurface)
         {
    +      if (OnNewFrame != null)
    +      {
    +        try
    +        {
    +          OnNewFrame(width, height, arWidth, arHeight, pSurface);
    +        }
    +        catch(Exception)
    +        {
    +        }
    +      }
           // Is GetCurrentImage() requesting a frame grab?
           if (!grabSample)
           {
     

    tourettes

    Retired Team Member
  • Premium Supporter
  • January 7, 2005
    17,301
    4,800
    Just wondering if those automatic zoom mode plugins are experiencing from high CPU usage as well? In that case I would call it as a bug fix. But somehow I feel like I'm not able to vote as I have lobbyed such huge amount for the feature freeze :)
     

    gemx

    Retired Team Member
  • Premium Supporter
  • October 31, 2006
    1,972
    539
    Home Country
    Germany Germany
    Well, kind of. IFC plugin for example only calls the framegrabber once per second but that could nevertheless cause a CPU peak when watching HD content which could stall MP for some ms.
    Althought this doesn't sound much, just think about these timing issues we had lately with the EVR renderer. ;)

    I don't say that this caused this behaviour but there is a possibility that it could cause this or other timing issues
     

    tourettes

    Retired Team Member
  • Premium Supporter
  • January 7, 2005
    17,301
    4,800
    Well, kind of. IFC plugin for example only calls the framegrabber once per second but that could nevertheless cause a CPU peak when watching HD content which could stall MP for some ms.

    Yes, it would indeed cause same stuttering (only with smaller frequency).
     

    arion_p

    Retired Team Member
  • Premium Supporter
  • February 7, 2007
    3,373
    1,626
    Athens
    Home Country
    Greece Greece
    I think this has been discussed before. While I agree that this is a welcome improvement, it may also cause issues if the event handler takes too long. IIRC this is called from within PresentImage(), so a long running event handler could easily hinder rendering and cause stuttering. I believe we need to somehow limit how long the event handler will take to finish.

    Even though the event handler provides the ultimate flexibility it introduces a big risk. Perhaps instead of an event handler we could provide a limited set of frame pre-processing options that are known to be fast and safe.

    Just some thoughts ;)
     

    gemx

    Retired Team Member
  • Premium Supporter
  • October 31, 2006
    1,972
    539
    Home Country
    Germany Germany
    I already thought about something like "GetCurrentImage(Rectangle source,Rectangle target)" which would work but the problem with this is that you have to specify the dimensions when in OnFrame(...) which would result in every application that uses the GetCurrentImage(...) would get the same regions or worse different Rect request would be overriden by each other.

    So the only solution i found was to deliver the source surface untouched in the event handler so that every function that registered to it can make it's own resizing but that would on the other hand mean the might happen several resizes (copies from GPU -> CPU) for the same surface which is also bad.

    Best intermediate solution would be to always resize the image to something like 300*200 (keeping the AR) and then output it via GetCurrentImage.

    How about that?
     

    arion_p

    Retired Team Member
  • Premium Supporter
  • February 7, 2007
    3,373
    1,626
    Athens
    Home Country
    Greece Greece
    That sounds reasonable. At least for the type of plugins that currently exist, I think it would be enough.

    If need be it could be extended so plugins could "vote" for the minimum size each requires, then the image would be scaled to the max of the requests. But for now I think a fixed size is better.

    BTW: Can't the resize be done in the GPU (use the surface as a texture and draw it on a smaller secondary surface)? If that is possible, it would lower CPU usage even more.
     

    infinite.loop

    Retired Team Member
  • Premium Supporter
  • December 26, 2004
    16,163
    4,133
    127.0.0.1
    Home Country
    Austria Austria
    Evenen though the proposed change sounds easy (in the first post at least, not so much later), i want to say that "the rules apply to everyone".
    hwharmann was also not allowed to add his minor change for the clock feature. And others to add simmilar small new features since we have a featurefreeze.

    Please lets focus on 1.1.0 (RC1/final) or MP2. :)

    :D
     

    tourettes

    Retired Team Member
  • Premium Supporter
  • January 7, 2005
    17,301
    4,800
    BTW: Can't the resize be done in the GPU (use the surface as a texture and draw it on a smaller secondary surface)? If that is possible, it would lower CPU usage even more.

    Should be possible.
     

    tourettes

    Retired Team Member
  • Premium Supporter
  • January 7, 2005
    17,301
    4,800
    I think this has been discussed before. While I agree that this is a welcome improvement, it may also cause issues if the event handler takes too long. IIRC this is called from within PresentImage(), so a long running event handler could easily hinder rendering and cause stuttering. I believe we need to somehow limit how long the event handler will take to finish.

    I think the correct solution would be to force plugin to use a worked thread to do the analysis. So, to do it properly would require more changes than the proposed one -> voting for > 1.1.0 as target.
     

    Users who are viewing this thread

    Top Bottom