Bug 143615 - Impress: Crash when slide transition = 0sec ( steps in comment 10 )
Summary: Impress: Crash when slide transition = 0sec ( steps in comment 10 )
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
5.2 all versions
Hardware: All All
: medium normal
Assignee: Thorsten Behrens (allotropia)
URL:
Whiteboard: target:7.4.0 target:7.3.4 target:7.2....
Keywords: haveBacktrace
Depends on:
Blocks: Sidebar-Slide-Transition Slide-Transitions Crash
  Show dependency treegraph
 
Reported: 2021-07-29 20:20 UTC by BDF
Modified: 2022-04-20 10:07 UTC (History)
7 users (show)

See Also:
Crash report or crash signature: ["slideshow::internal::RandomWipe::operator()(double)"]


Attachments
LO Impress - bug 143615 - crash with slide transition at 0sec (13.44 KB, application/vnd.oasis.opendocument.presentation)
2021-07-29 20:22 UTC, BDF
Details
bt with debug symbols (4.79 KB, text/plain)
2021-07-30 08:22 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description BDF 2021-07-29 20:20:45 UTC
Description:
When the duration of the slide transition is set to 0 seconds, some transitions make Impress crash.

Steps to Reproduce:
1. Open Impress and add a slide transition
2. Select the stripe transition (the symbol shows vertical lines, de: "Balken")
3. Set the duration to 0 Sec (see also the attached file)

Actual Results:
Impress crashes immediately

Expected Results:
Not sure what a transition of 0sec should look like (maybe there should be a min duration time of 0.01 sec), but Impress should not crash.


Reproducible: Always


User Profile Reset: No



Additional Info:
Only some transitions cause the crash (eg. Resolve - de:Auflösen - seems to cause the same issue). Most of them seem to not cause a crash.

---- - - - - Version Info - - - - ----
Version: 7.1.4.2 (x64) / LibreOffice Community
Build ID: a529a4fab45b75fefc5b6226684193eb000654f6
CPU threads: 16; OS: Windows 10.0 Build 19042; UI render: Skia/Raster; VCL: win
Locale: de-AT (de_AT); UI: de-DE
Calc: CL
Comment 1 BDF 2021-07-29 20:22:56 UTC
Created attachment 173957 [details]
LO Impress - bug 143615 - crash with slide transition at 0sec

To crash Impress, select transition Lines (de: Balken) or Resolve (de: Auflösen) and set the duration to 0 sec.
Comment 2 BDF 2021-07-29 20:23:33 UTC
CRASH REPORT NOTE:
The crash report was uploaded to https://crashreport.libreoffice.org/stats/crash_details/ad9c6cbc-04da-42a2-bde2-1b3294494641
Comment 3 Julien Nabet 2021-07-30 08:22:22 UTC
Created attachment 173966 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today, I got a crash but at a different location.
Comment 4 BogdanB 2021-07-30 08:37:21 UTC
Confirm with
Version: 7.1.5.2 (x64) / LibreOffice Community
Build ID: 85f04e9f809797b8199d13c421bd8a2b025d52b5
CPU threads: 4; OS: Windows 10.0 Build 19043; UI render: Skia/Raster; VCL: win
Locale: ro-RO (ro_RO); UI: en-US
Calc: threaded
Comment 5 Julien Nabet 2021-07-30 08:46:12 UTC
Miklos/Thorsten/Tibor (I don't know if one of you know slideshow part, I just used git history):
The pb is when choosing 0, "mnMinSimpleDuration" has value 0 but is used at locations like:
 68                 nCurrElapsedTime / mnMinSimpleDuration );
 118             double nT( nCurrElapsedTime / mnMinSimpleDuration );
in slideshow/source/engine/activities/simplecontinuousactivitybase.cxx
See https://opengrok.libreoffice.org/xref/core/slideshow/source/engine/activities/simplecontinuousactivitybase.cxx?r=c82efb61#68

Now, I wonder if it really makes sense to choose 0 and if we shouldn't put minimum at 1.
I read https://www.w3.org/TR/SMIL/smil-timing.html#q3 since code references SMIL but it's not very clear:
"dur
    Specifies the simple duration.
    The attribute value may be any of the following:

    Clock-value
        Specifies the length of the simple duration, measured in element active time.
        Value must be greater than 0."  <--------- so can't be 0

"The min/max attributes provide the author with a way to control the lower and upper bound of the element active duration.

min
    Specifies the minimum value of the active duration.
    The attribute value may be either of the following:

    Clock-value
        Specifies the length of the minimum value of the active duration, measured in element active time.
        Value must be greater than or equal to 0.
    "media"
        Specifies the minimum value of the active duration as the intrinsic media duration. This is only valid for elements that define media.

If there is any error in the argument value syntax for min, the attribute will be ignored (as though it were not specified).

The default value for min is "0". This does not constrain the active duration at all."
so can be 0.


What do you think?
Comment 6 Miklos Vajna 2021-08-02 12:06:40 UTC
The function already returns 0 in various other error cases, you could probably do the same just before the div by 0 would happen. Unless Thorsten has a better suggestion. :-)
Comment 7 Julien Nabet 2021-08-02 15:53:30 UTC
(In reply to Miklos Vajna from comment #6)
> The function already returns 0 in various other error cases, you could
> probably do the same just before the div by 0 would happen. Unless Thorsten
> has a better suggestion. :-)

Have you got an example of these other locations?
Indeed, I git grepped in slideshow dir "mnMinDuration" and "mnMinSimpleDuration", I don't see any examples to prevent from a division by 0.
Comment 8 Miklos Vajna 2021-08-03 11:51:53 UTC
slideshow/source/engine/activities/simplecontinuousactivitybase.cxx:49 and slideshow/source/engine/activities/simplecontinuousactivitybase.cxx:93 already returns 0 in the error code, my suggestion is to do the same before the div-by-zero would happen.
Comment 9 Julien Nabet 2021-08-03 12:13:01 UTC
I tried this:
diff --git a/slideshow/source/engine/activities/simplecontinuousactivitybase.cxx b/slideshow/source/engine/activities/simplecontinuousactivitybase.cxx
index 9e23fc2c76c8..a90f93994f76 100644
--- a/slideshow/source/engine/activities/simplecontinuousactivitybase.cxx
+++ b/slideshow/source/engine/activities/simplecontinuousactivitybase.cxx
@@ -46,7 +46,7 @@ namespace slideshow::internal
         double SimpleContinuousActivityBase::calcTimeLag() const
         {
             ActivityBase::calcTimeLag();
-            if (! isActive())
+            if (! isActive() || !mnMinSimpleDuration)
                 return 0.0;
 
             // retrieve locally elapsed time
@@ -107,7 +107,7 @@ namespace slideshow::internal
         bool SimpleContinuousActivityBase::perform()
         {
             // call base class, for start() calls and end handling
-            if( !ActivityBase::perform() )
+            if( !ActivityBase::perform() || !mnMinSimpleDuration )
                 return false; // done, we're ended
 
 

Setting 0 first time was ok then I increased to 1 then decreased again to 0, it hanged.

It seems more complicated than I thought.
uncc myself, I can't help here.
Comment 10 Xisco Faulí 2021-08-09 09:41:10 UTC
Also reproducible in

Version: 5.2.0.0.alpha1+
Build ID: 5b168b3fa568e48e795234dc5fa454bf24c9805e
CPU Threads: 4; OS Version: Linux 5.7; UI Render: default; 
Locale: en-US (en_US.UTF-8)

Steps to reproduce:
1. Open Impress
2. Go to Slide Transition in the sidebar
3. Change duration to 0 seconds
4. Select Bars transition

-> Crash
Comment 11 johnks 2021-09-10 15:51:54 UTC
i confirm this is present for me

Version: 7.2.0.4 / LibreOffice Community
Build ID: 9a9c6381e3f7a62afc1329bd359cc48accb6435b
CPU threads: 4; OS: Linux 5.11; UI render: default; VCL: gtk3
Locale: en-IN (en_IN); UI: en-US
Flatpak
Calc: threaded
Comment 12 Commit Notification 2022-04-14 17:29:55 UTC
Thorsten Behrens committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/e1db8c27875eac73b1e619e4a23ecdb7a9924b61

Resolves: tdf#143615 clamp relative times to 1.0

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2022-04-15 16:07:00 UTC
Thorsten Behrens committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/01c5006db900e3911e6bf8cb7abc2935e8215ded

Resolves: tdf#143615 clamp relative times to 1.0

It will be available in 7.3.4.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2022-04-15 16:29:17 UTC
Thorsten Behrens committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/18ac22f8a24cb4b691d8c0269206355d0f484625

Resolves: tdf#143615 clamp relative times to 1.0

It will be available in 7.2.7.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Commit Notification 2022-04-20 10:07:59 UTC
Thorsten Behrens committed a patch related to this issue.
It has been pushed to "libreoffice-7-3-3":

https://git.libreoffice.org/core/commit/007e21825ca4093f39bcdca8f5c2a0e6df0c205e

Resolves: tdf#143615 clamp relative times to 1.0

It will be available in 7.3.3.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.