Re: Submitting SubtitleComposer for KDE Review
On 11/7/21 20:57, Albert Astals Cid wrote: El diumenge, 7 de novembre de 2021, a les 19:05:44 (CET), Mladen Milinkovic va escriure: Have updated the bug report address to bugs.kde.org (default) in KAboutData and project has been added. Should I do something else to complete the process? From my side I think you fixed everything I reported. I'd say wait a week more in case someone else has time to have a look, but if there's no more movement consider the review done and ask sysadmin to finish the move to its final location. great... thanks When opening an existing .srt, there is a few Subtitle::insertLine calls that end up calling Subtitle::processAction with the if(app()->subtitle() != this) situation. I think all those Actions leak, because you just call redo on them but they are not deleted by anyone, no? Yes they were leaking fixed them with 911b94b. There are still some definite leaks after closing application: - libfontconfig/QTextDocument (FcFontRenderPrepare https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=655989) - QXcbGlxWindow::createVisual() calling radeon_dri.so and amdgpu_winsys_create() - Breeze::WidgetStateEngine::registerWidget calling QObject::connect (might be related to "QCoreApplication::postEvent: Unexpected null receiver" messages at application shutdown) - KF5WidgetAddons (KSelectActionPrivate::init()) I have found a cause of leak in KSelectActionPrivate::init(), it was also causing "QCoreApplication::postEvent: Unexpected null receiver" messages at application shutdown I fixed it inside SC, will submit a patch to invent.kde.org later.. I'm not sure what's the best why to fix it. -- Mladen Milinkovic GPG/PGP: EF9D9B26
Re: Submitting SubtitleComposer for KDE Review
Have updated the bug report address to bugs.kde.org (default) in KAboutData and project has been added. Should I do something else to complete the process? Cheers, Mladen On 10/16/21 00:28, Mladen Milinkovic wrote: On 10/15/21 22:39, Albert Astals Cid wrote: I think you may have dropped k-c-d from the CC, adding it back. I have hit a wrong button again :) El divendres, 15 d’octubre de 2021, a les 22:18:00 (CEST), Mladen Milinkovic va escriure: On 10/13/21 23:03, Albert Astals Cid wrote: I think the tests are somehow not correctly flagged as tests, running ctest will only run the appstream check and src/tests/test-subtitle, but not test-core-rangelist and the rest. Fixed in ec9ffba - I'm pretty sure this was working fine at some point in (not so distant) past. The first text format change doesn't seem to trigger the "file has changed and we should enable saving" logic. i.e. i have written a new subtitle line that says "HOLA" and saved the subtitle. Now if i select all the text and press the strikeout button, the save button does not get enabled, if i press the strikeout button again, the save button correctly gets enabled. I believe this was happening sometimes due to "relatively scary valgrind warning" below... looks like it's not happening anymore - could you please confirm? Seems to be working now :) If i close a video while it's playing, the Play button will still be enabled (if i stop the video it will not) Fixed in 663d209 Opening a .srt i just created and editing one of the subtitle lines i get this relatively scary valgrind warninghttps://ghostbin.com/YGnmL Fixed in 663d209. QUndoStack::push(action) can merge and delete action, in those cases it ended with invalid read immediately afterwards. When opening an existing .srt, there is a few Subtitle::insertLine calls that end up calling Subtitle::processAction with the if(app()->subtitle() != this) situation. I think all those Actions leak, because you just call redo on them but they are not deleted by anyone, no? Yes they were leaking fixed them with 911b94b. There are still some definite leaks after closing application: - libfontconfig/QTextDocument (FcFontRenderPrepare https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=655989) - QXcbGlxWindow::createVisual() calling radeon_dri.so and amdgpu_winsys_create() - Breeze::WidgetStateEngine::registerWidget calling QObject::connect (might be related to "QCoreApplication::postEvent: Unexpected null receiver" messages at application shutdown) - KF5WidgetAddons (KSelectActionPrivate::init()) There are also some memory errors that seem caused by KIO/KUrlRequester. There are some possible leaks related to QTextDocument and rendering, will investigate ASAP if it's due to something that SC does wrong. I think most of those are not your fault, but if you can spend a bit of time investigating won't hurt :) Sure... will do that at some point soonish. The "Report bug" incorrectly links tohttps://invent.kde.org/multimedia/subtitlecomposer/-/issues instead of bug.kde.org I didn't change the bug report url to bugs.kde.org yet as it doesn't seem possible to file Subtitle Composer bugs there? Would prefer to change it right before SC gets included there if it's necessary. There are SC binaries that get generated pretty often that people are using - I'd like them to have a bug report url they can use to report bugs. Ok, then should open a request at https://phabricator.kde.org/maniphest/task/edit/form/2/ so that a subtitlecomposer product is created :) And done https://phabricator.kde.org/T14947 Cheers, Albert Cheers, Albert Thank you! Cheers -- Mladen Milinkovic GPG/PGP: EF9D9B26
Re: Submitting SubtitleComposer for KDE Review
El diumenge, 7 de novembre de 2021, a les 19:05:44 (CET), Mladen Milinkovic va escriure: > Have updated the bug report address to bugs.kde.org (default) in KAboutData > and project has been added. > > Should I do something else to complete the process? From my side I think you fixed everything I reported. I'd say wait a week more in case someone else has time to have a look, but if there's no more movement consider the review done and ask sysadmin to finish the move to its final location. Cheers, Albert > > Cheers, > Mladen > > On 10/16/21 00:28, Mladen Milinkovic wrote: > > On 10/15/21 22:39, Albert Astals Cid wrote: > >> I think you may have dropped k-c-d from the CC, adding it back. > > > > I have hit a wrong button again :) > > > > > >> El divendres, 15 d’octubre de 2021, a les 22:18:00 (CEST), Mladen > >> Milinkovic va escriure: > >>> On 10/13/21 23:03, Albert Astals Cid wrote: > I think the tests are somehow not correctly flagged as tests, running > ctest will only run the appstream check and > src/tests/test-subtitle, but not test-core-rangelist and the rest. > >>> > >>> Fixed in ec9ffba - I'm pretty sure this was working fine at some point in > >>> (not so distant) past. > >>> > >>> > The first text format change doesn't seem to trigger the "file has > changed and we should enable saving" logic. i.e. > i have written a new subtitle line that says "HOLA" and saved the > subtitle. Now if i select all the text and press > the strikeout button, the save button does not get enabled, if i press > the strikeout button again, the save button > correctly gets enabled. > >>> > >>> I believe this was happening sometimes due to "relatively scary valgrind > >>> warning" below... looks like it's not > >>> happening > >>> anymore - could you please confirm? > >> > >> Seems to be working now :) > >> > >>> > >>> > If i close a video while it's playing, the Play button will still be > enabled (if i stop the video it will not) > >>> > >>> Fixed in 663d209 > >>> > >>> > Opening a .srt i just created and editing one of the subtitle lines i > get this relatively scary valgrind > warninghttps://ghostbin.com/YGnmL > >>> > >>> Fixed in 663d209. QUndoStack::push(action) can merge and delete action, > >>> in those cases it ended with invalid read > >>> immediately afterwards. > >>> > >>> > When opening an existing .srt, there is a few Subtitle::insertLine calls > that end up calling > Subtitle::processAction with the if(app()->subtitle() != this) > situation. I think all those Actions leak, because > you just call redo on them but they are not deleted by anyone, no? > >>> > >>> Yes they were leaking fixed them with 911b94b. > >>> > >>> There are still some definite leaks after closing application: > >>>- libfontconfig/QTextDocument (FcFontRenderPrepare > >>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=655989) > >>>- QXcbGlxWindow::createVisual() calling radeon_dri.so and > >>> amdgpu_winsys_create() > >>>- Breeze::WidgetStateEngine::registerWidget calling QObject::connect > >>> (might be related to > >>> "QCoreApplication::postEvent: Unexpected null receiver" messages at > >>> application shutdown) > >>>- KF5WidgetAddons (KSelectActionPrivate::init()) > >>> > >>> There are also some memory errors that seem caused by KIO/KUrlRequester. > >>> > >>> There are some possible leaks related to QTextDocument and rendering, > >>> will investigate ASAP if it's due to something > >>> that SC does wrong. > >> > >> I think most of those are not your fault, but if you can spend a bit of > >> time investigating won't hurt :) > > > > Sure... will do that at some point soonish. > > > > > The "Report bug" incorrectly links > tohttps://invent.kde.org/multimedia/subtitlecomposer/-/issues instead of > bug.kde.org > >>> > >>> I didn't change the bug report url to bugs.kde.org yet as it doesn't seem > >>> possible to file Subtitle Composer bugs > >>> there? > >>> Would prefer to change it right before SC gets included there if it's > >>> necessary. There are SC binaries that get > >>> generated pretty often that people are using - I'd like them to have a > >>> bug report url they can use to report bugs. > >> > >> Ok, then should open a request at > >> https://phabricator.kde.org/maniphest/task/edit/form/2/ so that a > >> subtitlecomposer > >> product is created :) > > > > And done https://phabricator.kde.org/T14947 > > > > > >> Cheers, > >>Albert > >> > >>> > > Cheers, > Albert > >>> > >>> Thank you! > >>> > >> > > > > Cheers > > >
Re: Submitting SubtitleComposer for KDE Review
On 10/15/21 22:39, Albert Astals Cid wrote: I think you may have dropped k-c-d from the CC, adding it back. I have hit a wrong button again :) El divendres, 15 d’octubre de 2021, a les 22:18:00 (CEST), Mladen Milinkovic va escriure: On 10/13/21 23:03, Albert Astals Cid wrote: I think the tests are somehow not correctly flagged as tests, running ctest will only run the appstream check and src/tests/test-subtitle, but not test-core-rangelist and the rest. Fixed in ec9ffba - I'm pretty sure this was working fine at some point in (not so distant) past. The first text format change doesn't seem to trigger the "file has changed and we should enable saving" logic. i.e. i have written a new subtitle line that says "HOLA" and saved the subtitle. Now if i select all the text and press the strikeout button, the save button does not get enabled, if i press the strikeout button again, the save button correctly gets enabled. I believe this was happening sometimes due to "relatively scary valgrind warning" below... looks like it's not happening anymore - could you please confirm? Seems to be working now :) If i close a video while it's playing, the Play button will still be enabled (if i stop the video it will not) Fixed in 663d209 Opening a .srt i just created and editing one of the subtitle lines i get this relatively scary valgrind warninghttps://ghostbin.com/YGnmL Fixed in 663d209. QUndoStack::push(action) can merge and delete action, in those cases it ended with invalid read immediately afterwards. When opening an existing .srt, there is a few Subtitle::insertLine calls that end up calling Subtitle::processAction with the if(app()->subtitle() != this) situation. I think all those Actions leak, because you just call redo on them but they are not deleted by anyone, no? Yes they were leaking fixed them with 911b94b. There are still some definite leaks after closing application: - libfontconfig/QTextDocument (FcFontRenderPrepare https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=655989) - QXcbGlxWindow::createVisual() calling radeon_dri.so and amdgpu_winsys_create() - Breeze::WidgetStateEngine::registerWidget calling QObject::connect (might be related to "QCoreApplication::postEvent: Unexpected null receiver" messages at application shutdown) - KF5WidgetAddons (KSelectActionPrivate::init()) There are also some memory errors that seem caused by KIO/KUrlRequester. There are some possible leaks related to QTextDocument and rendering, will investigate ASAP if it's due to something that SC does wrong. I think most of those are not your fault, but if you can spend a bit of time investigating won't hurt :) Sure... will do that at some point soonish. The "Report bug" incorrectly links tohttps://invent.kde.org/multimedia/subtitlecomposer/-/issues instead of bug.kde.org I didn't change the bug report url to bugs.kde.org yet as it doesn't seem possible to file Subtitle Composer bugs there? Would prefer to change it right before SC gets included there if it's necessary. There are SC binaries that get generated pretty often that people are using - I'd like them to have a bug report url they can use to report bugs. Ok, then should open a request at https://phabricator.kde.org/maniphest/task/edit/form/2/ so that a subtitlecomposer product is created :) And done https://phabricator.kde.org/T14947 Cheers, Albert Cheers, Albert Thank you! Cheers -- Mladen Milinkovic GPG/PGP: EF9D9B26
Re: Submitting SubtitleComposer for KDE Review
On 10/13/21 23:03, Albert Astals Cid wrote: I think the tests are somehow not correctly flagged as tests, running ctest will only run the appstream check and src/tests/test-subtitle, but not test-core-rangelist and the rest. Fixed in ec9ffba - I'm pretty sure this was working fine at some point in (not so distant) past. The first text format change doesn't seem to trigger the "file has changed and we should enable saving" logic. i.e. i have written a new subtitle line that says "HOLA" and saved the subtitle. Now if i select all the text and press the strikeout button, the save button does not get enabled, if i press the strikeout button again, the save button correctly gets enabled. I believe this was happening sometimes due to "relatively scary valgrind warning" below... looks like it's not happening anymore - could you please confirm? If i close a video while it's playing, the Play button will still be enabled (if i stop the video it will not) Fixed in 663d209 Opening a .srt i just created and editing one of the subtitle lines i get this relatively scary valgrind warninghttps://ghostbin.com/YGnmL Fixed in 663d209. QUndoStack::push(action) can merge and delete action, in those cases it ended with invalid read immediately afterwards. When opening an existing .srt, there is a few Subtitle::insertLine calls that end up calling Subtitle::processAction with the if(app()->subtitle() != this) situation. I think all those Actions leak, because you just call redo on them but they are not deleted by anyone, no? Yes they were leaking fixed them with 911b94b. There are still some definite leaks after closing application: - libfontconfig/QTextDocument (FcFontRenderPrepare https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=655989) - QXcbGlxWindow::createVisual() calling radeon_dri.so and amdgpu_winsys_create() - Breeze::WidgetStateEngine::registerWidget calling QObject::connect (might be related to "QCoreApplication::postEvent: Unexpected null receiver" messages at application shutdown) - KF5WidgetAddons (KSelectActionPrivate::init()) There are also some memory errors that seem caused by KIO/KUrlRequester. There are some possible leaks related to QTextDocument and rendering, will investigate ASAP if it's due to something that SC does wrong. The "Report bug" incorrectly links tohttps://invent.kde.org/multimedia/subtitlecomposer/-/issues instead of bug.kde.org I didn't change the bug report url to bugs.kde.org yet as it doesn't seem possible to file Subtitle Composer bugs there? Would prefer to change it right before SC gets included there if it's necessary. There are SC binaries that get generated pretty often that people are using - I'd like them to have a bug report url they can use to report bugs. Cheers, Albert Thank you! -- Mladen Milinkovic GPG/PGP: EF9D9B26
Re: Submitting SubtitleComposer for KDE Review
I think you may have dropped k-c-d from the CC, adding it back. El divendres, 15 d’octubre de 2021, a les 22:18:00 (CEST), Mladen Milinkovic va escriure: > On 10/13/21 23:03, Albert Astals Cid wrote: > > I think the tests are somehow not correctly flagged as tests, running ctest > > will only run the appstream check and src/tests/test-subtitle, but not > > test-core-rangelist and the rest. > > Fixed in ec9ffba - I'm pretty sure this was working fine at some point in > (not so distant) past. > > > > The first text format change doesn't seem to trigger the "file has changed > > and we should enable saving" logic. i.e. i have written a new subtitle line > > that says "HOLA" and saved the subtitle. Now if i select all the text and > > press the strikeout button, the save button does not get enabled, if i > > press the strikeout button again, the save button correctly gets enabled. > > I believe this was happening sometimes due to "relatively scary valgrind > warning" below... looks like it's not happening > anymore - could you please confirm? Seems to be working now :) > > > > If i close a video while it's playing, the Play button will still be > > enabled (if i stop the video it will not) > > Fixed in 663d209 > > > > Opening a .srt i just created and editing one of the subtitle lines i get > > this relatively scary valgrind warninghttps://ghostbin.com/YGnmL > > Fixed in 663d209. QUndoStack::push(action) can merge and delete action, in > those cases it ended with invalid read > immediately afterwards. > > > > When opening an existing .srt, there is a few Subtitle::insertLine calls > > that end up calling Subtitle::processAction with the if(app()->subtitle() > > != this) situation. I think all those Actions leak, because you just call > > redo on them but they are not deleted by anyone, no? > > Yes they were leaking fixed them with 911b94b. > > There are still some definite leaks after closing application: > - libfontconfig/QTextDocument (FcFontRenderPrepare > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=655989) > - QXcbGlxWindow::createVisual() calling radeon_dri.so and > amdgpu_winsys_create() > - Breeze::WidgetStateEngine::registerWidget calling QObject::connect (might > be related to > "QCoreApplication::postEvent: Unexpected null receiver" messages at > application shutdown) > - KF5WidgetAddons (KSelectActionPrivate::init()) > > There are also some memory errors that seem caused by KIO/KUrlRequester. > > There are some possible leaks related to QTextDocument and rendering, will > investigate ASAP if it's due to something > that SC does wrong. I think most of those are not your fault, but if you can spend a bit of time investigating won't hurt :) > > > The "Report bug" incorrectly links > > tohttps://invent.kde.org/multimedia/subtitlecomposer/-/issues instead of > > bug.kde.org > > I didn't change the bug report url to bugs.kde.org yet as it doesn't seem > possible to file Subtitle Composer bugs there? > Would prefer to change it right before SC gets included there if it's > necessary. There are SC binaries that get > generated pretty often that people are using - I'd like them to have a bug > report url they can use to report bugs. Ok, then should open a request at https://phabricator.kde.org/maniphest/task/edit/form/2/ so that a subtitlecomposer product is created :) Cheers, Albert > > > > > Cheers, > >Albert > > Thank you! >
Re: Submitting SubtitleComposer for KDE Review
El dimecres, 13 d’octubre de 2021, a les 17:23:54 (CEST), Mladen Milinkovic va escriure: > Hello everyone, > > I would like to move subtitlecomposer to kdereview. > > https://invent.kde.org/multimedia/subtitlecomposer > > https://subtitlecomposer.kde.org/ > > Subtitle Composer is an app to create, sync, translate, ... text subtitles > for videos and media. I think the tests are somehow not correctly flagged as tests, running ctest will only run the appstream check and src/tests/test-subtitle, but not test-core-rangelist and the rest. The first text format change doesn't seem to trigger the "file has changed and we should enable saving" logic. i.e. i have written a new subtitle line that says "HOLA" and saved the subtitle. Now if i select all the text and press the strikeout button, the save button does not get enabled, if i press the strikeout button again, the save button correctly gets enabled. If i close a video while it's playing, the Play button will still be enabled (if i stop the video it will not) Opening a .srt i just created and editing one of the subtitle lines i get this relatively scary valgrind warning https://ghostbin.com/YGnmL When opening an existing .srt, there is a few Subtitle::insertLine calls that end up calling Subtitle::processAction with the if(app()->subtitle() != this) situation. I think all those Actions leak, because you just call redo on them but they are not deleted by anyone, no? The "Report bug" incorrectly links to https://invent.kde.org/multimedia/subtitlecomposer/-/issues instead of bug.kde.org Cheers, Albert > > Thanks >
Submitting SubtitleComposer for KDE Review
Hello everyone, I would like to move subtitlecomposer to kdereview. https://invent.kde.org/multimedia/subtitlecomposer https://subtitlecomposer.kde.org/ Subtitle Composer is an app to create, sync, translate, ... text subtitles for videos and media. Thanks -- Mladen Milinkovic GPG/PGP: EF9D9B26
