xbmc / inputstream.adaptive

kodi inputstream addon for several manifest types
Other
453 stars 242 forks source link

(German-?) Streams with subtitles hang in the last few minutes #1417

Closed t0mcat1337 closed 11 months ago

t0mcat1337 commented 12 months ago

Bug report

Describe the bug

(German-?) Streams played through inputstream.adaptive suddenly hang (as if they where paused) somewhen in the last few minutes. One can stop them, continue them at the hanging position, they continue a few seconds, then hang again.

Reproducable examples are the german SoapOpera "Gute Zeiten - schlechte Zeiten", which can be played through the tvnow - plugin from the kodinerds repository.

Expected Behavior

Streams should be played without hanging

Actual Behavior

Streams hang in the last few minutes.

Possible Fix

All "hanging" streams have in common, that they contain subtitles, read below.

Debuglog

When streams hang, such entries can be found in kodi.log

2023-11-09 18:40:43.712 T:2366727   error <general>: AddOnLog: inputstream.adaptive: Download failed with error 404: https://vodnowusoawsdash.secure.footprint.net/p112/cves/sd/rtlplus/927829/2-1-1-1-2.ism/dash/2-1-1-1-2-textstream_eng=2000-682.m4s
2023-11-09 18:40:44.212 T:2366727 warning <general>: AddOnLog: inputstream.adaptive: Segment download failed, attempt 2...
2023-11-09 18:40:44.236 T:2366727   error <general>: AddOnLog: inputstream.adaptive: Download failed with error 404: https://vodnowusoawsdash.secure.footprint.net/p112/cves/sd/rtlplus/927829/2-1-1-1-2.ism/dash/2-1-1-1-2-textstream_eng=2000-682.m4s
2023-11-09 18:40:44.736 T:2366727 warning <general>: AddOnLog: inputstream.adaptive: Segment download failed, attempt 3...
2023-11-09 18:40:44.760 T:2366727   error <general>: AddOnLog: inputstream.adaptive: Download failed with error 404: https://vodnowusoawsdash.secure.footprint.net/p112/cves/sd/rtlplus/927829/2-1-1-1-2.ism/dash/2-1-1-1-2-textstream_eng=2000-682.m4s
2023-11-09 18:40:45.260 T:2366727 warning <general>: AddOnLog: inputstream.adaptive: Segment download failed, attempt 4...
2023-11-09 18:40:45.289 T:2366727   error <general>: AddOnLog: inputstream.adaptive: Download failed with error 404: https://vodnowusoawsdash.secure.footprint.net/p112/cves/sd/rtlplus/927829/2-1-1-1-2.ism/dash/2-1-1-1-2-textstream_eng=2000-682.m4s
2023-11-09 18:40:45.789 T:2366727 warning <general>: AddOnLog: inputstream.adaptive: Segment download failed, attempt 5...
2023-11-09 18:40:45.812 T:2366727   error <general>: AddOnLog: inputstream.adaptive: Download failed with error 404: https://vodnowusoawsdash.secure.footprint.net/p112/cves/sd/rtlplus/927829/2-1-1-1-2.ism/dash/2-1-1-1-2-textstream_eng=2000-682.m4s
2023-11-09 18:40:46.312 T:2366727 warning <general>: AddOnLog: inputstream.adaptive: Segment download failed, attempt 6...
2023-11-09 18:40:46.335 T:2366727   error <general>: AddOnLog: inputstream.adaptive: Download failed with error 404: https://vodnowusoawsdash.secure.footprint.net/p112/cves/sd/rtlplus/927829/2-1-1-1-2.ism/dash/2-1-1-1-2-textstream_eng=2000-682.m4s

MPD/M3U8s/ISM

https://vodnowusoawsdash.secure.footprint.net/p112/cves/sd/rtlplus/927829/2-1-1-1-2.ism/v1.mpd

<?xml version="1.0" encoding="utf-8"?>
<!-- Created with Unified Streaming Platform  (version=1.12.3-28597) -->
<MPD
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xmlns="urn:mpeg:dash:schema:mpd:2011"
  xmlns:cenc="urn:mpeg:cenc:2013"
  xmlns:mspr="urn:microsoft:playready"
  xsi:schemaLocation="urn:mpeg:dash:schema:mpd:2011 http://standards.iso.org/ittf/PubliclyAvailableStandards/MPEG-DASH_schema_files/DASH-MPD.xsd"
  type="static"
  mediaPresentationDuration="PT23M11.520S"
  maxSegmentDuration="PT3S"
  minBufferTime="PT10S"
  profiles="urn:mpeg:dash:profile:isoff-live:2011">
  <Period
    id="1"
    duration="PT23M11.520S">
    <BaseURL>dash/</BaseURL>
    <AdaptationSet
      id="1"
      group="1"
      contentType="audio"
      lang="de"
      segmentAlignment="true"
      audioSamplingRate="48000"
      mimeType="audio/mp4"
      codecs="mp4a.40.2"
      startWithSAP="1">
      <AudioChannelConfiguration
        schemeIdUri="urn:mpeg:dash:23003:3:audio_channel_configuration:2011"
        value="2" />
      <!-- Common Encryption -->
      <ContentProtection
        schemeIdUri="urn:mpeg:dash:mp4protection:2011"
        value="cenc"
        cenc:default_KID="AB2F23F0-34E6-1076-2E07-8C8D6AA52AC9">
      </ContentProtection>
      <!-- PlayReady -->
      <ContentProtection
        schemeIdUri="urn:uuid:9A04F079-9840-4286-AB92-E65BE0885F95"
        value="MSPR 2.0">
        <cenc:pssh>AAACyHBzc2gAAAAAmgTweZhAQoarkuZb4IhflQAAAqioAgAAAQABAJ4CPABXAFIATQBIAEUAQQBEAEUAUgAgAHgAbQBsAG4AcwA9ACIAaAB0AHQAcAA6AC8ALwBzAGMAaABlAG0AYQBzAC4AbQBpAGMAcgBvAHMAbwBmAHQALgBjAG8AbQAvAEQAUgBNAC8AMgAwADAANwAvADAAMwAvAFAAbABhAHkAUgBlAGEAZAB5AEgAZQBhAGQAZQByACIAIAB2AGUAcgBzAGkAbwBuAD0AIgA0AC4AMAAuADAALgAwACIAPgA8AEQAQQBUAEEAPgA8AFAAUgBPAFQARQBDAFQASQBOAEYATwA+ADwASwBFAFkATABFAE4APgAxADYAPAAvAEsARQBZAEwARQBOAD4APABBAEwARwBJAEQAPgBBAEUAUwBDAFQAUgA8AC8AQQBMAEcASQBEAD4APAAvAFAAUgBPAFQARQBDAFQASQBOAEYATwA+ADwASwBJAEQAPgA4AEMATQB2AHEAKwBZADAAZABoAEEAdQBCADQAeQBOAGEAcQBVAHEAeQBRAD0APQA8AC8ASwBJAEQAPgA8AEMASABFAEMASwBTAFUATQA+AFUAQgA4AG0AeQBKAGoARgBqAG8ANAA9ADwALwBDAEgARQBDAEsAUwBVAE0APgA8AEwAQQBfAFUAUgBMAD4AaAB0AHQAcABzADoALwAvAHAAbABhAHkAcgBlAGEAZAB5AC0AYwBvAHIAZQAuAHQAdgBuAG8AdwAuAGQAZQAvAHAAbABhAHkAcgBlAGEAZAB5AC8AYQBwAGkALwBSAGkAZwBoAHQAcwBNAGEAbgBhAGcAZQByAC4AYQBzAG0AeAA8AC8ATABBAF8AVQBSAEwAPgA8AC8ARABBAFQAQQA+ADwALwBXAFIATQBIAEUAQQBEAEUAUgA+AA==</cenc:pssh>
        <mspr:IsEncrypted>1</mspr:IsEncrypted>
        <mspr:IV_Size>8</mspr:IV_Size>
        <mspr:kid>8CMvq+Y0dhAuB4yNaqUqyQ==</mspr:kid>
        <mspr:pro>qAIAAAEAAQCeAjwAVwBSAE0ASABFAEEARABFAFIAIAB4AG0AbABuAHMAPQAiAGgAdAB0AHAAOgAvAC8AcwBjAGgAZQBtAGEAcwAuAG0AaQBjAHIAbwBzAG8AZgB0AC4AYwBvAG0ALwBEAFIATQAvADIAMAAwADcALwAwADMALwBQAGwAYQB5AFIAZQBhAGQAeQBIAGUAYQBkAGUAcgAiACAAdgBlAHIAcwBpAG8AbgA9ACIANAAuADAALgAwAC4AMAAiAD4APABEAEEAVABBAD4APABQAFIATwBUAEUAQwBUAEkATgBGAE8APgA8AEsARQBZAEwARQBOAD4AMQA2ADwALwBLAEUAWQBMAEUATgA+ADwAQQBMAEcASQBEAD4AQQBFAFMAQwBUAFIAPAAvAEEATABHAEkARAA+ADwALwBQAFIATwBUAEUAQwBUAEkATgBGAE8APgA8AEsASQBEAD4AOABDAE0AdgBxACsAWQAwAGQAaABBAHUAQgA0AHkATgBhAHEAVQBxAHkAUQA9AD0APAAvAEsASQBEAD4APABDAEgARQBDAEsAUwBVAE0APgBVAEIAOABtAHkASgBqAEYAagBvADQAPQA8AC8AQwBIAEUAQwBLAFMAVQBNAD4APABMAEEAXwBVAFIATAA+AGgAdAB0AHAAcwA6AC8ALwBwAGwAYQB5AHIAZQBhAGQAeQAtAGMAbwByAGUALgB0AHYAbgBvAHcALgBkAGUALwBwAGwAYQB5AHIAZQBhAGQAeQAvAGEAcABpAC8AUgBpAGcAaAB0AHMATQBhAG4AYQBnAGUAcgAuAGEAcwBtAHgAPAAvAEwAQQBfAFUAUgBMAD4APAAvAEQAQQBUAEEAPgA8AC8AVwBSAE0ASABFAEEARABFAFIAPgA=</mspr:pro>
      </ContentProtection>
      <!-- Widevine -->
      <ContentProtection
        schemeIdUri="urn:uuid:EDEF8BA9-79D6-4ACE-A3C8-27DCD51D21ED">
        <cenc:pssh>AAAAUHBzc2gAAAAA7e+LqXnWSs6jyCfc1R0h7QAAADAIARIgYWIyZjIzZjAzNGU2MTA3NjJlMDc4YzhkNmFhNTJhYzkiBjkyNzgyOSoCU0Q=</cenc:pssh>
      </ContentProtection>
      <Label>Deutsch</Label>
      <Role schemeIdUri="urn:mpeg:dash:role:2011" value="main" />
      <SegmentTemplate
        timescale="48000"
        duration="96000"
        initialization="2-1-1-1-2-$RepresentationID$.dash"
        media="2-1-1-1-2-$RepresentationID$-$Number$.m4s">
      </SegmentTemplate>
      <Representation
        id="audio_1=130000"
        bandwidth="130000">
      </Representation>
    </AdaptationSet>
    <AdaptationSet
      id="2"
      group="3"
      contentType="text"
      lang="de"
      mimeType="application/mp4"
      codecs="stpp.ttml.im1t"
      startWithSAP="1">
      <Label>Deutsch</Label>
      <Role schemeIdUri="urn:mpeg:dash:role:2011" value="subtitle" />
      <SegmentTemplate
        timescale="1000"
        duration="2000"
        endNumber="681"
        initialization="2-1-1-1-2-$RepresentationID$.dash"
        media="2-1-1-1-2-$RepresentationID$-$Number$.m4s">
      </SegmentTemplate>
      <Representation
        id="textstream_eng=2000"
        bandwidth="2000">
      </Representation>
    </AdaptationSet>
    <AdaptationSet
      id="3"
      group="2"
      contentType="video"
      par="16:9"
      segmentAlignment="true"
      sar="1:1"
      mimeType="video/mp4"
      startWithSAP="1">
      <!-- Common Encryption -->
      <ContentProtection
        schemeIdUri="urn:mpeg:dash:mp4protection:2011"
        value="cenc"
        cenc:default_KID="AB2F23F0-34E6-1076-2E07-8C8D6AA52AC9">
      </ContentProtection>
      <!-- PlayReady -->
      <ContentProtection
        schemeIdUri="urn:uuid:9A04F079-9840-4286-AB92-E65BE0885F95"
        value="MSPR 2.0">
        <cenc:pssh>AAACyHBzc2gAAAAAmgTweZhAQoarkuZb4IhflQAAAqioAgAAAQABAJ4CPABXAFIATQBIAEUAQQBEAEUAUgAgAHgAbQBsAG4AcwA9ACIAaAB0AHQAcAA6AC8ALwBzAGMAaABlAG0AYQBzAC4AbQBpAGMAcgBvAHMAbwBmAHQALgBjAG8AbQAvAEQAUgBNAC8AMgAwADAANwAvADAAMwAvAFAAbABhAHkAUgBlAGEAZAB5AEgAZQBhAGQAZQByACIAIAB2AGUAcgBzAGkAbwBuAD0AIgA0AC4AMAAuADAALgAwACIAPgA8AEQAQQBUAEEAPgA8AFAAUgBPAFQARQBDAFQASQBOAEYATwA+ADwASwBFAFkATABFAE4APgAxADYAPAAvAEsARQBZAEwARQBOAD4APABBAEwARwBJAEQAPgBBAEUAUwBDAFQAUgA8AC8AQQBMAEcASQBEAD4APAAvAFAAUgBPAFQARQBDAFQASQBOAEYATwA+ADwASwBJAEQAPgA4AEMATQB2AHEAKwBZADAAZABoAEEAdQBCADQAeQBOAGEAcQBVAHEAeQBRAD0APQA8AC8ASwBJAEQAPgA8AEMASABFAEMASwBTAFUATQA+AFUAQgA4AG0AeQBKAGoARgBqAG8ANAA9ADwALwBDAEgARQBDAEsAUwBVAE0APgA8AEwAQQBfAFUAUgBMAD4AaAB0AHQAcABzADoALwAvAHAAbABhAHkAcgBlAGEAZAB5AC0AYwBvAHIAZQAuAHQAdgBuAG8AdwAuAGQAZQAvAHAAbABhAHkAcgBlAGEAZAB5AC8AYQBwAGkALwBSAGkAZwBoAHQAcwBNAGEAbgBhAGcAZQByAC4AYQBzAG0AeAA8AC8ATABBAF8AVQBSAEwAPgA8AC8ARABBAFQAQQA+ADwALwBXAFIATQBIAEUAQQBEAEUAUgA+AA==</cenc:pssh>
        <mspr:IsEncrypted>1</mspr:IsEncrypted>
        <mspr:IV_Size>8</mspr:IV_Size>
        <mspr:kid>8CMvq+Y0dhAuB4yNaqUqyQ==</mspr:kid>
        <mspr:pro>qAIAAAEAAQCeAjwAVwBSAE0ASABFAEEARABFAFIAIAB4AG0AbABuAHMAPQAiAGgAdAB0AHAAOgAvAC8AcwBjAGgAZQBtAGEAcwAuAG0AaQBjAHIAbwBzAG8AZgB0AC4AYwBvAG0ALwBEAFIATQAvADIAMAAwADcALwAwADMALwBQAGwAYQB5AFIAZQBhAGQAeQBIAGUAYQBkAGUAcgAiACAAdgBlAHIAcwBpAG8AbgA9ACIANAAuADAALgAwAC4AMAAiAD4APABEAEEAVABBAD4APABQAFIATwBUAEUAQwBUAEkATgBGAE8APgA8AEsARQBZAEwARQBOAD4AMQA2ADwALwBLAEUAWQBMAEUATgA+ADwAQQBMAEcASQBEAD4AQQBFAFMAQwBUAFIAPAAvAEEATABHAEkARAA+ADwALwBQAFIATwBUAEUAQwBUAEkATgBGAE8APgA8AEsASQBEAD4AOABDAE0AdgBxACsAWQAwAGQAaABBAHUAQgA0AHkATgBhAHEAVQBxAHkAUQA9AD0APAAvAEsASQBEAD4APABDAEgARQBDAEsAUwBVAE0APgBVAEIAOABtAHkASgBqAEYAagBvADQAPQA8AC8AQwBIAEUAQwBLAFMAVQBNAD4APABMAEEAXwBVAFIATAA+AGgAdAB0AHAAcwA6AC8ALwBwAGwAYQB5AHIAZQBhAGQAeQAtAGMAbwByAGUALgB0AHYAbgBvAHcALgBkAGUALwBwAGwAYQB5AHIAZQBhAGQAeQAvAGEAcABpAC8AUgBpAGcAaAB0AHMATQBhAG4AYQBnAGUAcgAuAGEAcwBtAHgAPAAvAEwAQQBfAFUAUgBMAD4APAAvAEQAQQBUAEEAPgA8AC8AVwBSAE0ASABFAEEARABFAFIAPgA=</mspr:pro>
      </ContentProtection>
      <!-- Widevine -->
      <ContentProtection
        schemeIdUri="urn:uuid:EDEF8BA9-79D6-4ACE-A3C8-27DCD51D21ED">
        <cenc:pssh>AAAAUHBzc2gAAAAA7e+LqXnWSs6jyCfc1R0h7QAAADAIARIgYWIyZjIzZjAzNGU2MTA3NjJlMDc4YzhkNmFhNTJhYzkiBjkyNzgyOSoCU0Q=</cenc:pssh>
      </ContentProtection>
      <Role schemeIdUri="urn:mpeg:dash:role:2011" value="main" />
      <SegmentTemplate
        timescale="12800"
        duration="25600"
        initialization="2-1-1-1-2-$RepresentationID$.dash"
        media="2-1-1-1-2-$RepresentationID$-$Number$.m4s">
      </SegmentTemplate>
      <Representation
        id="video=336000"
        bandwidth="336000"
        width="640"
        height="360"
        codecs="avc1.4D401E"
        scanType="progressive">
      </Representation>
      <Representation
        id="video=659000"
        bandwidth="659000"
        width="768"
        height="432"
        codecs="avc1.4D401E"
        scanType="progressive">
      </Representation>
      <Representation
        id="video=1236000"
        bandwidth="1236000"
        width="960"
        height="540"
        codecs="avc1.4D401F"
        scanType="progressive">
      </Representation>
      <Representation
        id="video=2261000"
        bandwidth="2261000"
        width="1280"
        height="720"
        codecs="avc1.4D401F"
        scanType="progressive">
      </Representation>
      <Representation
        id="video=4004000"
        bandwidth="4004000"
        width="1920"
        height="1080"
        codecs="avc1.4D4028"
        scanType="progressive">
      </Representation>
      <Representation
        id="video=5793000"
        bandwidth="5793000"
        width="1920"
        height="1080"
        codecs="avc1.4D4028"
        scanType="progressive">
      </Representation>
    </AdaptationSet>
  </Period>
</MPD>

Now take a special look at this: In the AdaptationSet of the Subtitle (id="2", contentType="text"), there is the <SegmentTemplate> Tag with an Attribute endNumber="681"

I assume that this attribute is ignored by inputstream causing it trying to download a segment with number 682, which of course fails with 404 as can be seen in the log:

Download failed with error 404: https://vodnowusoawsdash.secure.footprint.net/p112/cves/sd/rtlplus/927829/2-1-1-1-2.ism/dash/2-1-1-1-2-textstream_eng=2000-682.m4s

At this moment the stream hangs.

Your Environment

Used Operating system:

CastagnaIT commented 12 months ago

your considerations are right there are many DASH features not implemented in ISA addon this is one then cause mentioned problems

the weird thing is that despite other players have endNumber attribute support to SegmentTemplate im not able to find this specification on the dash docs, look like a separate revision of this protocol that im not able to find

i think that with this we should also take a look for related things mentioned on DASH 4.4.3.5. Last Segment Message chapter at least for the Supplemental Descriptor with schemeIdUri http://dashif.org/guidelines/last-segment-number where we can also test with a sample stream: http://dash.akamaized.net/dash264/TestCasesIOP41/LastSegmentNumber/1/manifest_last_segment_num.mpd

t0mcat1337 commented 11 months ago

In the meantime, I've been looking into this issue a bit more, despite neither never really coded in C++ in general nor had seen ISA addons source code in special.

But even with my noob - knowledge at least I have managed: -> parsing endNumber Attribute from SegmentTemplate Tag in the dash XML -> Use it for not downloading segments beyond this endNumber, so the stream isn't hanging any more

I'm very sure especially the 2. point can be done much better with better knowledge of ISA code (and C++ ;) ), but I think it could at least be a start ;)

These changes actually are living in a branch of a forked ISA Repo from Omega branch in my GitHub Profile. Perhaps I could open a pull request?

BTW: I have tried "fixes" for Omega and Nexus Versions of ISA, which are very different because of the different ISA structures.

CastagnaIT commented 11 months ago

i havent had time to take a look in depth, your changes are a starting point but incomplete, we have to implement this in a more generic extensible way, because dash spec show more ways to signal last segment, your code is SegmentTemplate dependent, and so not correct and i also expect that with these changes to implement the Supplemental Descriptor

BTW: I have tried "fixes" for Omega and Nexus Versions of ISA, which are very different because of the different ISA structures.

true, many improvements has not been backported to Nexus due to heavy code changes but we treat this issue not as a bug, but an improvement, being it is an unsupported dash feature so in this case backport is not mandatory nor a priority, a partial backport may also be considered at later time

t0mcat1337 commented 11 months ago

I'm completely with you that my code is far away from being complete, but nevertheless it is a quick (and yes, dirty) fix/workaround for the specific problem with those streams which actually affects quite a few users. (Compare this thread in Kodinerds forum: https://www.kodinerds.net/thread/57646-release-tvnow-rtl-watchbox-vox-ntv-nowus-rtl2-nitro/)

Of course the complete, more generic and extensible solution you mention is neccessary, but it sounds that this will take some amount of time, especially when it comes to be backported / available in Nexus (which actually is the productive version)

Do u see any chance for a quicker (perhaps not as perfect) fix for this issue, so those streams work again for the currently affected users?

CastagnaIT commented 11 months ago

Do u see any chance for a quicker (perhaps not as perfect) fix for this issue, so those streams work again for the currently affected users?

often people want "quick fixes" but many "quick" fixes were introduced in the past that messed up the source code a lot, as you can see from differences between Omega VS Nexus branch that has required tons of refactors to give manageability to the code

who want merge a quick fix doesn't always mean it was done with broader thinking in mind, who develop a fast fix may not bother to complete the changes made at later time, then some other people will have to study and solve this a second time, or worst it will be forgotten and today small problem become a future huge problem that require big code refactor and many hours to spend

i noticed that you are able to understand a bit C++ and you might understand that the amount of work here is huge we are 2 devs with little time, tons of Issue threads to be solved and improvements "under the hood" to do

Perhaps I could open a pull request?

about this, anyone can participate to the source code, and we are happy to accept new guests even casual, this will help lighten our tasks!

who wants to participate to the project must observe the development rules and requests from project maintainers in general Kodi team devs, on the Wiki page there is "Contributing and code guidelines" section that is a starting point that i suggest you to read if you would like to take an interest in this

however i will start take a look this week, atm i dont comment about backport as said in my previous message

CastagnaIT commented 11 months ago

can you test the PR #1422? you can find also some test builds here: https://jenkins.kodi.tv/blue/organizations/jenkins/xbmc%2Finputstream.adaptive/detail/PR-1422/1/artifacts

t0mcat1337 commented 11 months ago

Just tested PR with a freshly built kodi omega from master and with this ISA the error is gone, streams are played without problems. Well done :)

CastagnaIT commented 11 months ago

since its small change i made backport for kodi 20 if you have chance to test builds: https://jenkins.kodi.tv/blue/organizations/jenkins/xbmc%2Finputstream.adaptive/detail/PR-1425/3/artifacts

t0mcat1337 commented 11 months ago

Awesome...as I just found artifacts for ios, android, windows, osx and tvos in your jenkins - link but no linux one, I compiled your branch end_segment_nexus on my own and tested this. With this version, streams are playing without any problem in Nexus, too.

t0mcat1337 commented 11 months ago

I'm afraid I have some bad news... I just tested the Nexus Backport in IAS Nexus Branch and this doesn't fix the issue... ISA still tries downloading SubTitles Segments beyond the endNumber...

t0mcat1337 commented 11 months ago

OK, don't really know what I messed up during the test but this obviously was my mistake, big sorry for that. I debugged this in VSCode and found the buggy code in DASHTree.cpp lines 1279ff:

When EndNumer is set in Template: image image --> Correct countSegs from Templates EndNumber: 681

But right after, we enter the "else" branch because of wrong logical if ... else structure: image image

and then the countSegs is overwritten: image

Working solution for me was simply inserting "else if...": image

CastagnaIT commented 11 months ago

thanks to have find out my oversight!